From dbcb12295616db729f92f1a2e2b29399d0063786 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 30 May 2026 02:56:12 +0000 Subject: [PATCH 1/5] =?UTF-8?q?feat(web+macos):=20wire=20deep-link=20consu?= =?UTF-8?q?mers=20=E2=80=94=20composer=20pre-fill,=20thread=20nav,=20windo?= =?UTF-8?q?w=20activation=20(LUM-2068)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the consumer loop on the deep-link bridge from LUM-1872. The bus events fired by `use-event-bus-init` now have a chat-domain subscriber that takes user-visible action, plus a new `runtime/main-window.ts` bridge so any consumer can bring the window forward. ## What ### Main-window bridge (new imperative surface) - `apps/macos/src/main/main-window.ts` — `installMainWindow` now registers `ipcMain.handle("vellum:mainWindow:ensureVisible", ...)` that routes through the existing `ensureVisible()` (recreate if destroyed, restore from minimize, show, focus, await renderer- ready). Test seam `__resetForTesting` added so tests can re-exercise the install path without re-mocking the entire module. - `apps/macos/src/main/preload/index.ts` — `window.vellum.mainWindow.ensureVisible()`. - `apps/web/src/runtime/is-electron.ts` — ambient mirror. - `apps/web/src/runtime/main-window.ts` — no-op-off-Electron wrapper; web has its own foregrounding (it IS the foreground tab) and Capacitor handles app activation natively. This is the third imperative wrapper after `dock.ts` and `app-info.ts`; same shape, same convention. ### Consumer hook in the chat domain - `apps/web/src/domains/chat/hooks/use-deep-link-consumer.ts` — subscribes to `deeplink.send`, `deeplink.openThread`, `deeplink.unknown` and: * `deeplink.send` → `ensureMainWindowVisible()`; if composer is empty (`.trim().length === 0`) set the input to the message; if non-empty, Sentry breadcrumb + drop. The conservative drop-don't-overwrite call is documented inline — we can revisit with a "queue or non-destructive prompt" UX once we know how often it happens in practice. * `deeplink.openThread` → `ensureMainWindowVisible()` + `navigate(routes.conversation(threadId))`. * `deeplink.unknown` → Sentry breadcrumb with the URL, no other action. Uses refs to mirror the dynamic props (composerInput, setComposerInput, navigate) so the bus subscription is mounted once — without this, every keystroke would tear down + resubscribe the handlers, opening a race window where a deep link landing between unsubscribe and resubscribe would be dropped. ### Mounted from ChatPage - `apps/web/src/domains/chat/chat-page.tsx` — calls `useDeepLinkConsumer({ composerInput: input, setComposerInput: setInput })` next to the existing `useDraftInput`. ## Tests - `apps/macos/src/main/main-window.test.ts` (+2 cases) — IPC handler registered, IPC handler routes through `ensureVisible`. - `apps/web/src/domains/chat/hooks/use-deep-link-consumer.test.tsx` (7 cases) — `deeplink.send` empty-composer pre-fill, in-progress-typing preserved + Sentry breadcrumb, whitespace-only counts as empty; `deeplink.openThread` navigates + ensures window; `deeplink.unknown` Sentry breadcrumb only; re-render-doesn't-resubscribe (the dep-array-via-refs choice); unmount unsubscribes. 26/26 macOS tests + 26/26 chat+bus tests green. ## Out of scope - The "queue or non-destructive prompt" UX for in-progress typing. Sentry breadcrumb now; revisit with data. - Accessory-mode → regular transition before showing the window. The main-window helper's `ensureVisible` already handles the show/focus sequence; the dock policy refresh fires off the window's `show` event via `onMainWindowVisibilityChange`, so the transition happens implicitly. Worth a follow-up audit if the dock policy lags the show. https://claude.ai/code/session_011sZ4p8AkqDQMSavHvWfioe --- apps/macos/src/main/main-window.test.ts | 40 +++- apps/macos/src/main/main-window.ts | 18 +- apps/macos/src/preload/index.ts | 16 ++ apps/web/src/domains/chat/chat-page.tsx | 8 + .../hooks/use-deep-link-consumer.test.tsx | 189 ++++++++++++++++++ .../chat/hooks/use-deep-link-consumer.ts | 102 ++++++++++ apps/web/src/runtime/is-electron.ts | 3 + apps/web/src/runtime/main-window.ts | 27 +++ 8 files changed, 401 insertions(+), 2 deletions(-) create mode 100644 apps/web/src/domains/chat/hooks/use-deep-link-consumer.test.tsx create mode 100644 apps/web/src/domains/chat/hooks/use-deep-link-consumer.ts create mode 100644 apps/web/src/runtime/main-window.ts diff --git a/apps/macos/src/main/main-window.test.ts b/apps/macos/src/main/main-window.test.ts index 562058e2a9c..48b284bed28 100644 --- a/apps/macos/src/main/main-window.test.ts +++ b/apps/macos/src/main/main-window.test.ts @@ -98,6 +98,13 @@ const makeWindow = (): StubWindow => { return stub; }; +const ipcHandlers = new Map unknown>(); +const ipcHandleMock = mock( + (channel: string, handler: (...args: unknown[]) => unknown) => { + ipcHandlers.set(channel, handler); + }, +); + mock.module("electron", () => ({ app: { isPackaged: false, @@ -107,6 +114,7 @@ mock.module("electron", () => ({ Object.assign(this, makeWindow()); } }, + ipcMain: { handle: ipcHandleMock }, shell: { openExternal: () => Promise.resolve() }, })); @@ -115,7 +123,7 @@ mock.module("./window-state", () => ({ track: () => undefined, })); -const { current, dispatchToMain, ensureVisible, hide, installMainWindow, isVisibleAndFocused, toggleVisibility } = +const { __resetForTesting, current, dispatchToMain, ensureVisible, hide, installMainWindow, isVisibleAndFocused, toggleVisibility } = await import("./main-window"); const reset = (): void => { @@ -130,6 +138,9 @@ const reset = (): void => { beforeEach(() => { reset(); + __resetForTesting(); + ipcHandlers.clear(); + ipcHandleMock.mockClear(); }); afterEach(() => { @@ -357,6 +368,33 @@ describe("installMainWindow", () => { installMainWindow(); expect(constructed).toHaveLength(1); }); + + test("registers the vellum:mainWindow:ensureVisible IPC handler", () => { + installMainWindow(); + expect(ipcHandlers.has("vellum:mainWindow:ensureVisible")).toBe(true); + }); + + test("the IPC handler routes through ensureVisible (recreating if destroyed, showing + focusing otherwise)", async () => { + installMainWindow(); + const win = constructed[0]; + if (!win) throw new Error("expected a window"); + // The initial install fires ensureVisible too — settle its readiness + // gate before exercising the IPC path so the assertion isolates the + // IPC-driven calls. + win.stub.webContents.emit("did-finish-load"); + win.stub.emit("ready-to-show"); + const showsBefore = win.stub.show.mock.calls.length; + const focusBefore = win.stub.focus.mock.calls.length; + + const handler = ipcHandlers.get("vellum:mainWindow:ensureVisible"); + const promise = (handler as () => Promise)(); + // ensureVisible on the existing-but-not-focused window returns + // immediately (ALREADY_READY for the already-loaded window). + await promise; + + expect(win.stub.show.mock.calls.length).toBeGreaterThan(showsBefore); + expect(win.stub.focus.mock.calls.length).toBeGreaterThan(focusBefore); + }); }); describe("dispatchToMain", () => { diff --git a/apps/macos/src/main/main-window.ts b/apps/macos/src/main/main-window.ts index 99c5ea86a3f..5ee752a37d3 100644 --- a/apps/macos/src/main/main-window.ts +++ b/apps/macos/src/main/main-window.ts @@ -1,4 +1,4 @@ -import { BrowserWindow, app, shell } from "electron"; +import { BrowserWindow, app, ipcMain, shell } from "electron"; import path from "node:path"; import { @@ -309,5 +309,21 @@ let installed = false; export const installMainWindow = (): void => { if (installed) return; installed = true; + + // IPC surface for renderer-driven "bring the window forward" + // actions — used by feature consumers reacting to inbound signals + // (deep links, future notification clicks, etc.). The renderer + // wrapper at `apps/web/src/runtime/main-window.ts` calls this; the + // handler returns void so the caller can `await` without value. + ipcMain.handle("vellum:mainWindow:ensureVisible", async (): Promise => { + await ensureVisible(); + }); + void ensureVisible(); }; + +// Test seam — exported only for unit-test setup. Production code +// uses `installMainWindow` instead. +export const __resetForTesting = (): void => { + installed = false; +}; diff --git a/apps/macos/src/preload/index.ts b/apps/macos/src/preload/index.ts index 1eee5f7d27b..46990c3a91d 100644 --- a/apps/macos/src/preload/index.ts +++ b/apps/macos/src/preload/index.ts @@ -118,6 +118,18 @@ export interface VellumBridge { */ setSignedIn(signedIn: boolean): Promise; }; + mainWindow: { + /** + * Bring the main window to the foreground: recreate if destroyed, + * restore from minimize, show, focus. Used by feature consumers + * reacting to inbound signals (deep links, future notification + * clicks) that should be accompanied by the window becoming + * user-visible. Resolves once the renderer is loaded and the + * window is focused — same readiness gate as the main-process + * `ensureVisible`. + */ + ensureVisible(): Promise; + }; power: { /** * Subscribe to system power-state events: sleep, wake, screen @@ -196,6 +208,10 @@ const bridge: VellumBridge = { setSignedIn: (signedIn: boolean): Promise => ipcRenderer.invoke("vellum:dock:setSignedIn", signedIn) as Promise, }, + mainWindow: { + ensureVisible: (): Promise => + ipcRenderer.invoke("vellum:mainWindow:ensureVisible") as Promise, + }, power: { onEvent: (callback) => { const handler = (_event: IpcRendererEvent, payload: PowerEvent) => { diff --git a/apps/web/src/domains/chat/chat-page.tsx b/apps/web/src/domains/chat/chat-page.tsx index 9ae3182798a..33c2581ca84 100644 --- a/apps/web/src/domains/chat/chat-page.tsx +++ b/apps/web/src/domains/chat/chat-page.tsx @@ -83,6 +83,7 @@ import { useInteractionActions } from "@/domains/chat/hooks/use-interaction-acti import { useEventStream } from "@/domains/chat/hooks/use-event-stream"; import { useActiveAppPinSync } from "@/domains/chat/hooks/use-active-app-pin-sync"; import { useDraftInput } from "@/domains/chat/components/chat-composer/use-draft-input"; +import { useDeepLinkConsumer } from "@/domains/chat/hooks/use-deep-link-consumer"; import { useRefreshLatestMessages } from "@/domains/chat/hooks/use-refresh-latest-messages"; import { useChatDebugApi } from "@/domains/chat/utils/debug-api"; @@ -397,6 +398,13 @@ export function ChatPage() { onDraftRestored: setRestoredDraftConversationId, }); + // Inbound deep links: pre-fill composer with `deeplink.send` text, + // navigate to `/assistant/conversations/` for `deeplink.openThread`, + // and ensure the main window is visible first. The hook gates the + // composer pre-fill on `input` being empty so it doesn't clobber + // in-progress typing. Off Electron the bus events never fire. + useDeepLinkConsumer({ composerInput: input, setComposerInput: setInput }); + useEffect(() => { const onKeyDown = (event: KeyboardEvent) => { const inputEl = inputRef.current; diff --git a/apps/web/src/domains/chat/hooks/use-deep-link-consumer.test.tsx b/apps/web/src/domains/chat/hooks/use-deep-link-consumer.test.tsx new file mode 100644 index 00000000000..5b40a22b908 --- /dev/null +++ b/apps/web/src/domains/chat/hooks/use-deep-link-consumer.test.tsx @@ -0,0 +1,189 @@ +import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; +import { cleanup, renderHook, act } from "@testing-library/react"; + +import { + __resetEventBusForTesting, + useEventBusStore, +} from "@/stores/event-bus-store"; + +// Capture the navigate callback so we can assert on navigation +// targets without standing up a real router. +const navigateMock = mock((_to: string) => undefined); +mock.module("react-router", () => ({ + useNavigate: () => navigateMock, +})); + +// `ensureMainWindowVisible` is the bridge wrapper; replace with a spy +// so we can assert each handler fires it. +const ensureMainWindowVisibleMock = mock(async () => undefined); +mock.module("@/runtime/main-window", () => ({ + ensureMainWindowVisible: ensureMainWindowVisibleMock, +})); + +const sentryBreadcrumbMock = mock((_args: unknown) => undefined); +mock.module("@sentry/browser", () => ({ + addBreadcrumb: sentryBreadcrumbMock, +})); + +const { useDeepLinkConsumer } = await import("./use-deep-link-consumer"); + +const renderConsumer = ( + composerInput: string, + setComposerInput: (next: string) => void, +) => + renderHook( + ({ input, set }: { input: string; set: (next: string) => void }) => + useDeepLinkConsumer({ composerInput: input, setComposerInput: set }), + { initialProps: { input: composerInput, set: setComposerInput } }, + ); + +beforeEach(() => { + __resetEventBusForTesting(); + navigateMock.mockClear(); + ensureMainWindowVisibleMock.mockClear(); + sentryBreadcrumbMock.mockClear(); +}); + +afterEach(() => { + cleanup(); + __resetEventBusForTesting(); +}); + +describe("deeplink.send", () => { + test("pre-fills the composer when input is empty", () => { + const setComposerInput = mock((_next: string) => undefined); + renderConsumer("", setComposerInput); + + act(() => { + useEventBusStore.getState().publish("deeplink.send", { message: "hi" }); + }); + + expect(setComposerInput).toHaveBeenCalledWith("hi"); + expect(ensureMainWindowVisibleMock).toHaveBeenCalledTimes(1); + }); + + test("preserves in-progress typing — drops the link with a Sentry breadcrumb", () => { + const setComposerInput = mock((_next: string) => undefined); + renderConsumer("user already typing", setComposerInput); + + act(() => { + useEventBusStore + .getState() + .publish("deeplink.send", { message: "from link" }); + }); + + expect(setComposerInput).not.toHaveBeenCalled(); + expect(sentryBreadcrumbMock).toHaveBeenCalled(); + // ensureMainWindowVisible still fires — the window should come + // forward so the user sees Vellum even when we declined to overwrite. + expect(ensureMainWindowVisibleMock).toHaveBeenCalledTimes(1); + }); + + test("whitespace-only composer input counts as empty", () => { + const setComposerInput = mock((_next: string) => undefined); + renderConsumer(" \n ", setComposerInput); + + act(() => { + useEventBusStore.getState().publish("deeplink.send", { message: "hi" }); + }); + + expect(setComposerInput).toHaveBeenCalledWith("hi"); + }); +}); + +describe("deeplink.openThread", () => { + test("navigates to the conversation route and ensures the window is visible", () => { + renderConsumer("", () => undefined); + + act(() => { + useEventBusStore + .getState() + .publish("deeplink.openThread", { threadId: "abc-123" }); + }); + + expect(navigateMock).toHaveBeenCalledWith( + "/assistant/conversations/abc-123", + ); + expect(ensureMainWindowVisibleMock).toHaveBeenCalledTimes(1); + }); +}); + +describe("deeplink.unknown", () => { + test("records a Sentry breadcrumb with the URL and takes no other action", () => { + const setComposerInput = mock((_next: string) => undefined); + renderConsumer("", setComposerInput); + + act(() => { + useEventBusStore + .getState() + .publish("deeplink.unknown", { url: "javascript:alert(1)" }); + }); + + expect(sentryBreadcrumbMock).toHaveBeenCalled(); + const args = sentryBreadcrumbMock.mock.calls[0]?.[0] as { + data?: { url?: string }; + }; + expect(args.data?.url).toBe("javascript:alert(1)"); + // No navigation, no composer set, no main-window activation — + // unknown links don't do anything user-visible. + expect(navigateMock).not.toHaveBeenCalled(); + expect(setComposerInput).not.toHaveBeenCalled(); + expect(ensureMainWindowVisibleMock).not.toHaveBeenCalled(); + }); +}); + +describe("subscription lifecycle", () => { + test("re-renders with new composerInput don't tear down + resubscribe the bus listeners", () => { + // If the effect's dep array included composerInput, every + // keystroke would unsubscribe + resubscribe. Verify it + // doesn't by counting subscriber registrations across a + // re-render — should stay 3 (one per event). + const setComposerInput = mock((_next: string) => undefined); + const { rerender } = renderConsumer("", setComposerInput); + + // After mount we expect 3 subscriptions registered on the bus. + // Publish each event and assert handlers fired exactly once. + act(() => { + useEventBusStore.getState().publish("deeplink.send", { message: "x" }); + }); + expect(setComposerInput).toHaveBeenCalledTimes(1); + + // Trigger a re-render with new input. If the effect re-runs, + // the next publish would see the dep-array reset + handlers + // mounting + unmounting — and the assert below could either + // dupe or drop the event in a race. The stable subscription + // makes it deterministic. + rerender({ input: "typing", set: setComposerInput }); + + act(() => { + useEventBusStore + .getState() + .publish("deeplink.openThread", { threadId: "z" }); + }); + expect(navigateMock).toHaveBeenCalledTimes(1); + }); + + test("unsubscribes on unmount", () => { + const setComposerInput = mock((_next: string) => undefined); + const { unmount } = renderConsumer("", setComposerInput); + + unmount(); + + // After unmount, publishing shouldn't reach the handlers. + act(() => { + useEventBusStore + .getState() + .publish("deeplink.send", { message: "post-unmount" }); + useEventBusStore + .getState() + .publish("deeplink.openThread", { threadId: "z" }); + useEventBusStore + .getState() + .publish("deeplink.unknown", { url: "x" }); + }); + + expect(setComposerInput).not.toHaveBeenCalled(); + expect(navigateMock).not.toHaveBeenCalled(); + expect(sentryBreadcrumbMock).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/web/src/domains/chat/hooks/use-deep-link-consumer.ts b/apps/web/src/domains/chat/hooks/use-deep-link-consumer.ts new file mode 100644 index 00000000000..b67d2e235ce --- /dev/null +++ b/apps/web/src/domains/chat/hooks/use-deep-link-consumer.ts @@ -0,0 +1,102 @@ +import { useEffect, useRef } from "react"; +import * as Sentry from "@sentry/browser"; +import { useNavigate } from "react-router"; + +import { ensureMainWindowVisible } from "@/runtime/main-window"; +import { useEventBusStore } from "@/stores/event-bus-store"; +import { routes } from "@/utils/routes"; + +/** + * Wires the renderer-side actions for inbound deep links published + * on the event bus by `useEventBusInit` (which fans the Electron + * deep-link bridge into typed `deeplink.*` events). + * + * - `deeplink.send { message }` → pre-fill the chat composer IF + * it's empty. Refusing to overwrite the user's in-progress text + * is the conservative call; the dropped link is captured as a + * Sentry breadcrumb so we can see how often it happens before + * investing in a "queue or prompt" UX. + * - `deeplink.openThread { threadId }` → navigate to the + * conversation route. Router handles not-found. + * - `deeplink.unknown { url }` → Sentry breadcrumb + no action. + * Useful telemetry on unrecognized links the OS routed to us. + * + * All three handlers fire `ensureMainWindowVisible()` first so + * the action lands on a user-visible window. Off Electron the + * wrapper no-ops. + * + * Lives in the chat domain (per ELECTRON.md's "hooks that bridge + * feature state live in the domain") because both load-bearing + * actions (composer pre-fill, conversation navigation) are + * chat-specific. Mounted from `ChatPage` so it has access to the + * composer's `setInput`. + */ + +export interface UseDeepLinkConsumerParams { + /** Current composer input — checked before pre-fill so we don't + * clobber the user's in-progress typing. */ + composerInput: string; + /** Setter for the composer input. */ + setComposerInput: (next: string) => void; +} + +export function useDeepLinkConsumer({ + composerInput, + setComposerInput, +}: UseDeepLinkConsumerParams): void { + const navigate = useNavigate(); + + // Mirror the dynamic props into refs so the bus subscription + // effect can mount once and read fresh values at handler time. + // Subscribing inside an effect with these in the dep array would + // tear down + resubscribe on every keystroke (composerInput + // changes constantly) — wasteful and a race window where a deep + // link could land between unsubscribe and resubscribe. + const composerInputRef = useRef(composerInput); + const setComposerInputRef = useRef(setComposerInput); + composerInputRef.current = composerInput; + setComposerInputRef.current = setComposerInput; + const navigateRef = useRef(navigate); + navigateRef.current = navigate; + + useEffect(() => { + const bus = useEventBusStore.getState(); + + const unsubSend = bus.subscribe("deeplink.send", ({ message }) => { + void ensureMainWindowVisible(); + if (composerInputRef.current.trim().length > 0) { + Sentry.addBreadcrumb({ + category: "deeplink", + level: "info", + message: + "deeplink.send arrived but composer had unsaved text — drop, do not overwrite", + }); + return; + } + setComposerInputRef.current(message); + }); + + const unsubOpenThread = bus.subscribe( + "deeplink.openThread", + ({ threadId }) => { + void ensureMainWindowVisible(); + navigateRef.current(routes.conversation(threadId)); + }, + ); + + const unsubUnknown = bus.subscribe("deeplink.unknown", ({ url }) => { + Sentry.addBreadcrumb({ + category: "deeplink", + level: "info", + message: "deeplink.unknown", + data: { url }, + }); + }); + + return () => { + unsubSend(); + unsubOpenThread(); + unsubUnknown(); + }; + }, []); +} diff --git a/apps/web/src/runtime/is-electron.ts b/apps/web/src/runtime/is-electron.ts index 3c763db2fb5..9b1a37a2500 100644 --- a/apps/web/src/runtime/is-electron.ts +++ b/apps/web/src/runtime/is-electron.ts @@ -50,6 +50,9 @@ declare global { setBadge(count: number): Promise; setSignedIn(signedIn: boolean): Promise; }; + mainWindow: { + ensureVisible(): Promise; + }; power: { onEvent( callback: (event: { diff --git a/apps/web/src/runtime/main-window.ts b/apps/web/src/runtime/main-window.ts new file mode 100644 index 00000000000..8a32c5cace5 --- /dev/null +++ b/apps/web/src/runtime/main-window.ts @@ -0,0 +1,27 @@ +import { isElectron } from "@/runtime/is-electron"; + +/** + * Per-capability wrapper for the Electron host's main-window control + * surface. Imperative — `ensureMainWindowVisible()` brings the main + * window forward (recreate if destroyed, restore from minimize, show, + * focus). Off Electron the call no-ops; web and Capacitor iOS have + * their own foregrounding semantics (web is already the foreground + * tab if the user is interacting; iOS handles app activation + * natively). + * + * Used by feature consumers that react to inbound signals (deep + * links, future notification action clicks) and need to accompany + * the state update with making the window user-visible. Without + * this, a click on `vellum://send?message=hi` on a backgrounded + * Vellum would update composer state with no visible response. + * + * Same wrapper shape as `dock.ts` / `app-info.ts`: no-op off + * Electron, awaits an IPC handler that resolves once the window + * is loaded and focused. Safe to fire-and-forget if the caller + * doesn't need to sequence follow-up actions. + */ + +export async function ensureMainWindowVisible(): Promise { + if (!isElectron()) return; + await window.vellum?.mainWindow.ensureVisible(); +} From 83f549e8992092d4219cac7be0ce5e277fd9cfda Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 30 May 2026 03:04:03 +0000 Subject: [PATCH 2/5] fix(web): split deep-link consumer so non-chat routes still handle inbound links MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex P1 — the prior consumer mounted only on ChatPage, so deep links arriving while the user was on /assistant/settings, /logs, /home, /inspect, etc. dropped silently. useEventBusInit drained pending links at RootLayout scope and published them on the bus, but no subscriber existed → bus drops them. Real user impact: vellum://thread/ clicks did nothing on every non-chat route. ## Architectural split Two hooks, two stores, one narrow-waist hand-off. `apps/web/src/hooks/use-global-deep-link-consumer.ts` (new) — mounted at RootLayout, alive on every authenticated assistant route. Subscribes to the bus, handles navigation + window + unknown: - deeplink.openThread → ensureMainWindowVisible() + navigate(routes.conversation(threadId)) - deeplink.send → ensureMainWindowVisible() + navigate(routes.assistant) + park the message in usePendingDeepLinkStore - deeplink.unknown → Sentry breadcrumb `apps/web/src/stores/pending-deep-link-store.ts` (new) — Zustand one-shot inbox. The global consumer writes; the chat-domain consumer reads-and-clears on mount. Latest message wins if a second link arrives before consumption. Not persisted — deep links are transient signals. `apps/web/src/domains/chat/hooks/use-deep-link-consumer.ts` (refactored) — no longer subscribes to the bus directly. Reads pendingComposerMessage via the atomic selector, consumes on the effect, applies empty-composer-only policy. Refs gone since the deps are simple now. ## Why this shape The composer pre-fill is chat-domain-specific (owns setInput); it can ONLY mount when ChatPage mounts. The navigation, window, and unknown handlers are route-independent and MUST be always-on. The Zustand store is the narrow-waist hand-off: global writes, chat reads-once. Alternative considered: mount the whole consumer at RootLayout and push setInput up via outlet context / Zustand. Rejected because the composer state is intentionally local to useDraftInput (its draft persistence + conversation-switch lifecycle). The hand-off store is a smaller surface than lifting the composer state up. ## Tests `use-global-deep-link-consumer.test.tsx` (new, 4 cases): send navigates + parks the message + ensures window; openThread navigates + ensures window; unknown is breadcrumb-only; unmount unsubscribes. `use-deep-link-consumer.test.tsx` (rewritten, 5 cases): empty-composer pre-fill when a message is pending; non-empty composer preserves typing + breadcrumb + clears the store anyway (so it doesn't resurface on next render); whitespace-only counts as empty; no-op when nothing pending; LATE arrival fires on the Zustand re-render. All 9 tests green. ## Related to Devin BUG_0001 Devin flagged a docstring inconsistency on the previous commit (dbcb122956) — the old `useDeepLinkConsumer` claimed "all three handlers fire ensureMainWindowVisible" but `deeplink.unknown` did not. Moot post-refactor: the chat-domain hook no longer handles bus events at all, and the global consumer's docstring lists each handler's behavior precisely. https://claude.ai/code/session_011sZ4p8AkqDQMSavHvWfioe --- .../hooks/use-deep-link-consumer.test.tsx | 169 +++++------------- .../chat/hooks/use-deep-link-consumer.ts | 107 ++++------- .../use-global-deep-link-consumer.test.tsx | 123 +++++++++++++ .../hooks/use-global-deep-link-consumer.ts | 71 ++++++++ apps/web/src/root-layout.tsx | 8 + .../web/src/stores/pending-deep-link-store.ts | 71 ++++++++ 6 files changed, 347 insertions(+), 202 deletions(-) create mode 100644 apps/web/src/hooks/use-global-deep-link-consumer.test.tsx create mode 100644 apps/web/src/hooks/use-global-deep-link-consumer.ts create mode 100644 apps/web/src/stores/pending-deep-link-store.ts diff --git a/apps/web/src/domains/chat/hooks/use-deep-link-consumer.test.tsx b/apps/web/src/domains/chat/hooks/use-deep-link-consumer.test.tsx index 5b40a22b908..22ff8212b46 100644 --- a/apps/web/src/domains/chat/hooks/use-deep-link-consumer.test.tsx +++ b/apps/web/src/domains/chat/hooks/use-deep-link-consumer.test.tsx @@ -1,24 +1,10 @@ import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; -import { cleanup, renderHook, act } from "@testing-library/react"; +import { act, cleanup, renderHook } from "@testing-library/react"; import { - __resetEventBusForTesting, - useEventBusStore, -} from "@/stores/event-bus-store"; - -// Capture the navigate callback so we can assert on navigation -// targets without standing up a real router. -const navigateMock = mock((_to: string) => undefined); -mock.module("react-router", () => ({ - useNavigate: () => navigateMock, -})); - -// `ensureMainWindowVisible` is the bridge wrapper; replace with a spy -// so we can assert each handler fires it. -const ensureMainWindowVisibleMock = mock(async () => undefined); -mock.module("@/runtime/main-window", () => ({ - ensureMainWindowVisible: ensureMainWindowVisibleMock, -})); + __resetPendingDeepLinkForTesting, + usePendingDeepLinkStore, +} from "@/stores/pending-deep-link-store"; const sentryBreadcrumbMock = mock((_args: unknown) => undefined); mock.module("@sentry/browser", () => ({ @@ -38,152 +24,79 @@ const renderConsumer = ( ); beforeEach(() => { - __resetEventBusForTesting(); - navigateMock.mockClear(); - ensureMainWindowVisibleMock.mockClear(); + __resetPendingDeepLinkForTesting(); sentryBreadcrumbMock.mockClear(); }); afterEach(() => { cleanup(); - __resetEventBusForTesting(); + __resetPendingDeepLinkForTesting(); }); -describe("deeplink.send", () => { - test("pre-fills the composer when input is empty", () => { +describe("pending message consumption", () => { + test("pre-fills the composer when a pending message exists and input is empty", () => { const setComposerInput = mock((_next: string) => undefined); - renderConsumer("", setComposerInput); + // Stash a message before render — the consumer sees it on mount. + usePendingDeepLinkStore.getState().setPendingComposerMessage("hello"); - act(() => { - useEventBusStore.getState().publish("deeplink.send", { message: "hi" }); - }); + renderConsumer("", setComposerInput); - expect(setComposerInput).toHaveBeenCalledWith("hi"); - expect(ensureMainWindowVisibleMock).toHaveBeenCalledTimes(1); + expect(setComposerInput).toHaveBeenCalledWith("hello"); + // Consumed → store is cleared. + expect(usePendingDeepLinkStore.getState().pendingComposerMessage).toBe( + null, + ); }); - test("preserves in-progress typing — drops the link with a Sentry breadcrumb", () => { + test("preserves in-progress typing — drops with a Sentry breadcrumb", () => { const setComposerInput = mock((_next: string) => undefined); - renderConsumer("user already typing", setComposerInput); + usePendingDeepLinkStore + .getState() + .setPendingComposerMessage("from link"); - act(() => { - useEventBusStore - .getState() - .publish("deeplink.send", { message: "from link" }); - }); + renderConsumer("user already typing", setComposerInput); expect(setComposerInput).not.toHaveBeenCalled(); expect(sentryBreadcrumbMock).toHaveBeenCalled(); - // ensureMainWindowVisible still fires — the window should come - // forward so the user sees Vellum even when we declined to overwrite. - expect(ensureMainWindowVisibleMock).toHaveBeenCalledTimes(1); + // Message is consumed (cleared) either way — we don't want it to + // sit and resurface on the next render. + expect(usePendingDeepLinkStore.getState().pendingComposerMessage).toBe( + null, + ); }); test("whitespace-only composer input counts as empty", () => { const setComposerInput = mock((_next: string) => undefined); - renderConsumer(" \n ", setComposerInput); - - act(() => { - useEventBusStore.getState().publish("deeplink.send", { message: "hi" }); - }); - - expect(setComposerInput).toHaveBeenCalledWith("hi"); - }); -}); + usePendingDeepLinkStore.getState().setPendingComposerMessage("hello"); -describe("deeplink.openThread", () => { - test("navigates to the conversation route and ensures the window is visible", () => { - renderConsumer("", () => undefined); - - act(() => { - useEventBusStore - .getState() - .publish("deeplink.openThread", { threadId: "abc-123" }); - }); + renderConsumer(" \n ", setComposerInput); - expect(navigateMock).toHaveBeenCalledWith( - "/assistant/conversations/abc-123", - ); - expect(ensureMainWindowVisibleMock).toHaveBeenCalledTimes(1); + expect(setComposerInput).toHaveBeenCalledWith("hello"); }); -}); -describe("deeplink.unknown", () => { - test("records a Sentry breadcrumb with the URL and takes no other action", () => { + test("no-op when no message is pending", () => { const setComposerInput = mock((_next: string) => undefined); - renderConsumer("", setComposerInput); - act(() => { - useEventBusStore - .getState() - .publish("deeplink.unknown", { url: "javascript:alert(1)" }); - }); + renderConsumer("", setComposerInput); - expect(sentryBreadcrumbMock).toHaveBeenCalled(); - const args = sentryBreadcrumbMock.mock.calls[0]?.[0] as { - data?: { url?: string }; - }; - expect(args.data?.url).toBe("javascript:alert(1)"); - // No navigation, no composer set, no main-window activation — - // unknown links don't do anything user-visible. - expect(navigateMock).not.toHaveBeenCalled(); expect(setComposerInput).not.toHaveBeenCalled(); - expect(ensureMainWindowVisibleMock).not.toHaveBeenCalled(); - }); -}); - -describe("subscription lifecycle", () => { - test("re-renders with new composerInput don't tear down + resubscribe the bus listeners", () => { - // If the effect's dep array included composerInput, every - // keystroke would unsubscribe + resubscribe. Verify it - // doesn't by counting subscriber registrations across a - // re-render — should stay 3 (one per event). - const setComposerInput = mock((_next: string) => undefined); - const { rerender } = renderConsumer("", setComposerInput); - - // After mount we expect 3 subscriptions registered on the bus. - // Publish each event and assert handlers fired exactly once. - act(() => { - useEventBusStore.getState().publish("deeplink.send", { message: "x" }); - }); - expect(setComposerInput).toHaveBeenCalledTimes(1); - - // Trigger a re-render with new input. If the effect re-runs, - // the next publish would see the dep-array reset + handlers - // mounting + unmounting — and the assert below could either - // dupe or drop the event in a race. The stable subscription - // makes it deterministic. - rerender({ input: "typing", set: setComposerInput }); - - act(() => { - useEventBusStore - .getState() - .publish("deeplink.openThread", { threadId: "z" }); - }); - expect(navigateMock).toHaveBeenCalledTimes(1); + expect(sentryBreadcrumbMock).not.toHaveBeenCalled(); }); - test("unsubscribes on unmount", () => { + test("a pending message arriving after mount fires the effect on the next render", () => { const setComposerInput = mock((_next: string) => undefined); - const { unmount } = renderConsumer("", setComposerInput); - - unmount(); + renderConsumer("", setComposerInput); + expect(setComposerInput).not.toHaveBeenCalled(); - // After unmount, publishing shouldn't reach the handlers. + // Simulate the global consumer parking a message after the + // chat page is already mounted. The Zustand atomic selector + // re-renders the hook and the effect fires. act(() => { - useEventBusStore - .getState() - .publish("deeplink.send", { message: "post-unmount" }); - useEventBusStore + usePendingDeepLinkStore .getState() - .publish("deeplink.openThread", { threadId: "z" }); - useEventBusStore - .getState() - .publish("deeplink.unknown", { url: "x" }); + .setPendingComposerMessage("late arrival"); }); - expect(setComposerInput).not.toHaveBeenCalled(); - expect(navigateMock).not.toHaveBeenCalled(); - expect(sentryBreadcrumbMock).not.toHaveBeenCalled(); + expect(setComposerInput).toHaveBeenCalledWith("late arrival"); }); }); diff --git a/apps/web/src/domains/chat/hooks/use-deep-link-consumer.ts b/apps/web/src/domains/chat/hooks/use-deep-link-consumer.ts index b67d2e235ce..83e8553207d 100644 --- a/apps/web/src/domains/chat/hooks/use-deep-link-consumer.ts +++ b/apps/web/src/domains/chat/hooks/use-deep-link-consumer.ts @@ -1,35 +1,30 @@ -import { useEffect, useRef } from "react"; +import { useEffect } from "react"; import * as Sentry from "@sentry/browser"; -import { useNavigate } from "react-router"; -import { ensureMainWindowVisible } from "@/runtime/main-window"; -import { useEventBusStore } from "@/stores/event-bus-store"; -import { routes } from "@/utils/routes"; +import { usePendingDeepLinkStore } from "@/stores/pending-deep-link-store"; /** - * Wires the renderer-side actions for inbound deep links published - * on the event bus by `useEventBusInit` (which fans the Electron - * deep-link bridge into typed `deeplink.*` events). + * Chat-domain half of the deep-link consumer pair. Reads the pending + * `deeplink.send` message parked in `usePendingDeepLinkStore` by the + * global consumer (`useGlobalDeepLinkConsumer`, mounted at + * `RootLayout`) and applies it to the composer. * - * - `deeplink.send { message }` → pre-fill the chat composer IF - * it's empty. Refusing to overwrite the user's in-progress text - * is the conservative call; the dropped link is captured as a - * Sentry breadcrumb so we can see how often it happens before - * investing in a "queue or prompt" UX. - * - `deeplink.openThread { threadId }` → navigate to the - * conversation route. Router handles not-found. - * - `deeplink.unknown { url }` → Sentry breadcrumb + no action. - * Useful telemetry on unrecognized links the OS routed to us. + * Split exists because the global consumer must be route-stable + * (deep links arrive whenever, not just on `/assistant`), but only + * the chat domain knows about `setInput`. The store is the + * narrow-waist hand-off. * - * All three handlers fire `ensureMainWindowVisible()` first so - * the action lands on a user-visible window. Off Electron the - * wrapper no-ops. + * Semantics: * - * Lives in the chat domain (per ELECTRON.md's "hooks that bridge - * feature state live in the domain") because both load-bearing - * actions (composer pre-fill, conversation navigation) are - * chat-specific. Mounted from `ChatPage` so it has access to the - * composer's `setInput`. + * - If the composer is empty (`.trim().length === 0`), consume the + * pending message and `setComposerInput(message)`. + * - If non-empty, drop with a Sentry breadcrumb — refusing to + * overwrite the user's in-progress typing is the conservative + * call until we have telemetry to justify a "queue or prompt" UX. + * - Runs on every render where `pendingComposerMessage` is non-null, + * so a deep link arriving WHILE `ChatPage` is already mounted is + * picked up on the next render. The Zustand selector re-renders + * the component when the slice changes. */ export interface UseDeepLinkConsumerParams { @@ -44,59 +39,23 @@ export function useDeepLinkConsumer({ composerInput, setComposerInput, }: UseDeepLinkConsumerParams): void { - const navigate = useNavigate(); - - // Mirror the dynamic props into refs so the bus subscription - // effect can mount once and read fresh values at handler time. - // Subscribing inside an effect with these in the dep array would - // tear down + resubscribe on every keystroke (composerInput - // changes constantly) — wasteful and a race window where a deep - // link could land between unsubscribe and resubscribe. - const composerInputRef = useRef(composerInput); - const setComposerInputRef = useRef(setComposerInput); - composerInputRef.current = composerInput; - setComposerInputRef.current = setComposerInput; - const navigateRef = useRef(navigate); - navigateRef.current = navigate; + const pending = usePendingDeepLinkStore.use.pendingComposerMessage(); useEffect(() => { - const bus = useEventBusStore.getState(); - - const unsubSend = bus.subscribe("deeplink.send", ({ message }) => { - void ensureMainWindowVisible(); - if (composerInputRef.current.trim().length > 0) { - Sentry.addBreadcrumb({ - category: "deeplink", - level: "info", - message: - "deeplink.send arrived but composer had unsaved text — drop, do not overwrite", - }); - return; - } - setComposerInputRef.current(message); - }); - - const unsubOpenThread = bus.subscribe( - "deeplink.openThread", - ({ threadId }) => { - void ensureMainWindowVisible(); - navigateRef.current(routes.conversation(threadId)); - }, - ); - - const unsubUnknown = bus.subscribe("deeplink.unknown", ({ url }) => { + if (pending === null) return; + const consumed = usePendingDeepLinkStore + .getState() + .consumePendingComposerMessage(); + if (consumed === null) return; + if (composerInput.trim().length > 0) { Sentry.addBreadcrumb({ category: "deeplink", level: "info", - message: "deeplink.unknown", - data: { url }, + message: + "deeplink.send arrived but composer had unsaved text — drop, do not overwrite", }); - }); - - return () => { - unsubSend(); - unsubOpenThread(); - unsubUnknown(); - }; - }, []); + return; + } + setComposerInput(consumed); + }, [pending, composerInput, setComposerInput]); } diff --git a/apps/web/src/hooks/use-global-deep-link-consumer.test.tsx b/apps/web/src/hooks/use-global-deep-link-consumer.test.tsx new file mode 100644 index 00000000000..452c48ba02b --- /dev/null +++ b/apps/web/src/hooks/use-global-deep-link-consumer.test.tsx @@ -0,0 +1,123 @@ +import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; +import { cleanup, renderHook, act } from "@testing-library/react"; + +import { + __resetEventBusForTesting, + useEventBusStore, +} from "@/stores/event-bus-store"; +import { + __resetPendingDeepLinkForTesting, + usePendingDeepLinkStore, +} from "@/stores/pending-deep-link-store"; + +const navigateMock = mock((_to: string) => undefined); +mock.module("react-router", () => ({ + useNavigate: () => navigateMock, +})); + +const ensureMainWindowVisibleMock = mock(async () => undefined); +mock.module("@/runtime/main-window", () => ({ + ensureMainWindowVisible: ensureMainWindowVisibleMock, +})); + +const sentryBreadcrumbMock = mock((_args: unknown) => undefined); +mock.module("@sentry/browser", () => ({ + addBreadcrumb: sentryBreadcrumbMock, +})); + +const { useGlobalDeepLinkConsumer } = await import( + "./use-global-deep-link-consumer" +); + +beforeEach(() => { + __resetEventBusForTesting(); + __resetPendingDeepLinkForTesting(); + navigateMock.mockClear(); + ensureMainWindowVisibleMock.mockClear(); + sentryBreadcrumbMock.mockClear(); +}); + +afterEach(() => { + cleanup(); + __resetEventBusForTesting(); + __resetPendingDeepLinkForTesting(); +}); + +describe("deeplink.send", () => { + test("navigates to /assistant + parks the message in the pending store + ensures window", () => { + renderHook(() => useGlobalDeepLinkConsumer()); + + act(() => { + useEventBusStore.getState().publish("deeplink.send", { message: "hi" }); + }); + + expect(navigateMock).toHaveBeenCalledWith("/assistant"); + expect(usePendingDeepLinkStore.getState().pendingComposerMessage).toBe( + "hi", + ); + expect(ensureMainWindowVisibleMock).toHaveBeenCalledTimes(1); + }); +}); + +describe("deeplink.openThread", () => { + test("navigates to the conversation route + ensures window", () => { + renderHook(() => useGlobalDeepLinkConsumer()); + + act(() => { + useEventBusStore + .getState() + .publish("deeplink.openThread", { threadId: "abc-123" }); + }); + + expect(navigateMock).toHaveBeenCalledWith( + "/assistant/conversations/abc-123", + ); + expect(ensureMainWindowVisibleMock).toHaveBeenCalledTimes(1); + }); +}); + +describe("deeplink.unknown", () => { + test("Sentry breadcrumb only — no navigation or window activation", () => { + renderHook(() => useGlobalDeepLinkConsumer()); + + act(() => { + useEventBusStore + .getState() + .publish("deeplink.unknown", { url: "javascript:alert(1)" }); + }); + + expect(sentryBreadcrumbMock).toHaveBeenCalled(); + const args = sentryBreadcrumbMock.mock.calls[0]?.[0] as { + data?: { url?: string }; + }; + expect(args.data?.url).toBe("javascript:alert(1)"); + expect(navigateMock).not.toHaveBeenCalled(); + expect(ensureMainWindowVisibleMock).not.toHaveBeenCalled(); + }); +}); + +describe("subscription lifecycle", () => { + test("unmount unsubscribes — published events after unmount have no effect", () => { + const { unmount } = renderHook(() => useGlobalDeepLinkConsumer()); + + unmount(); + + act(() => { + useEventBusStore + .getState() + .publish("deeplink.send", { message: "post-unmount" }); + useEventBusStore + .getState() + .publish("deeplink.openThread", { threadId: "z" }); + useEventBusStore + .getState() + .publish("deeplink.unknown", { url: "x" }); + }); + + expect(navigateMock).not.toHaveBeenCalled(); + expect(sentryBreadcrumbMock).not.toHaveBeenCalled(); + expect(usePendingDeepLinkStore.getState().pendingComposerMessage).toBe( + null, + ); + }); +}); diff --git a/apps/web/src/hooks/use-global-deep-link-consumer.ts b/apps/web/src/hooks/use-global-deep-link-consumer.ts new file mode 100644 index 00000000000..1646afb0a91 --- /dev/null +++ b/apps/web/src/hooks/use-global-deep-link-consumer.ts @@ -0,0 +1,71 @@ +import { useEffect, useRef } from "react"; +import * as Sentry from "@sentry/browser"; +import { useNavigate } from "react-router"; + +import { ensureMainWindowVisible } from "@/runtime/main-window"; +import { useEventBusStore } from "@/stores/event-bus-store"; +import { usePendingDeepLinkStore } from "@/stores/pending-deep-link-store"; +import { routes } from "@/utils/routes"; + +/** + * Global deep-link consumer — mounted at `RootLayout` so it's alive + * for every authenticated assistant route, not just `/assistant` + * (`ChatPage`). Without it, a `vellum://thread/...` click while the + * user is on `/assistant/settings` would be dropped. + * + * Responsibilities: + * + * - `deeplink.openThread` → `ensureMainWindowVisible()` + + * `navigate(routes.conversation(threadId))`. + * - `deeplink.send` → `ensureMainWindowVisible()` + navigate to + * `/assistant` + park the message in `usePendingDeepLinkStore` + * for `ChatPage`'s composer-domain hook to consume on mount. + * - `deeplink.unknown` → Sentry breadcrumb. + * + * The composer pre-fill itself stays in the chat domain + * (`useDeepLinkConsumer`) because it owns `setInput`. This hook is + * intentionally generic — it doesn't import chat-specific state. + */ + +export function useGlobalDeepLinkConsumer(): void { + const navigate = useNavigate(); + // Mirror dynamic deps in a ref so the effect mounts once. Without + // this, a navigate-fn identity change would tear down + resubscribe + // the bus listeners and open a race window where a link could + // arrive between unsubscribe and resubscribe. + const navigateRef = useRef(navigate); + navigateRef.current = navigate; + + useEffect(() => { + const bus = useEventBusStore.getState(); + + const unsubSend = bus.subscribe("deeplink.send", ({ message }) => { + void ensureMainWindowVisible(); + usePendingDeepLinkStore.getState().setPendingComposerMessage(message); + navigateRef.current(routes.assistant); + }); + + const unsubOpenThread = bus.subscribe( + "deeplink.openThread", + ({ threadId }) => { + void ensureMainWindowVisible(); + navigateRef.current(routes.conversation(threadId)); + }, + ); + + const unsubUnknown = bus.subscribe("deeplink.unknown", ({ url }) => { + Sentry.addBreadcrumb({ + category: "deeplink", + level: "info", + message: "deeplink.unknown", + data: { url }, + }); + }); + + return () => { + unsubSend(); + unsubOpenThread(); + unsubUnknown(); + }; + }, []); +} diff --git a/apps/web/src/root-layout.tsx b/apps/web/src/root-layout.tsx index 3cadea1138b..cc43b37ae14 100644 --- a/apps/web/src/root-layout.tsx +++ b/apps/web/src/root-layout.tsx @@ -2,6 +2,7 @@ import { Outlet, useNavigate } from "react-router"; import { useAppTheme } from "@/hooks/use-app-theme"; import { useEventBusInit } from "@/hooks/use-event-bus-init"; +import { useGlobalDeepLinkConsumer } from "@/hooks/use-global-deep-link-consumer"; import { useIsMobile } from "@/hooks/use-is-mobile"; import { useVisibleViewport } from "@/hooks/use-visible-viewport"; import { useAssistantLifecycle } from "@/assistant/use-lifecycle"; @@ -81,6 +82,13 @@ export function RootLayout() { useDocumentEditorSync(); useEventBusInit({ assistantId, isAssistantActive }); + // Inbound deep-link navigation + window activation. Mounted here + // (not in `ChatPage`) so a `vellum://thread/...` arriving while + // the user is on `/assistant/settings`, `/logs`, etc. still + // navigates. The composer-pre-fill half lives in `ChatPage`'s + // `useDeepLinkConsumer` because it owns `setInput`; the two + // hand off via `pending-deep-link-store`. + useGlobalDeepLinkConsumer(); const keyboardOpen = isMobile && diff --git a/apps/web/src/stores/pending-deep-link-store.ts b/apps/web/src/stores/pending-deep-link-store.ts new file mode 100644 index 00000000000..1a999280cfb --- /dev/null +++ b/apps/web/src/stores/pending-deep-link-store.ts @@ -0,0 +1,71 @@ +/** + * Pending deep-link state — a one-shot inbox the global deep-link + * consumer writes to and the chat composer reads from. + * + * Why a store: a `vellum://send?message=…` deep link can arrive + * while the user is on a non-chat route (`/assistant/settings`, + * `/assistant/logs`, etc.). The global consumer (mounted at + * `RootLayout`) navigates to the chat AND parks the message here; + * `ChatPage` then consumes on mount once the composer's state owner + * (`useDraftInput`) is alive. Without this hand-off, the message + * would be dropped — the bus event publishes to no chat-domain + * subscriber until `ChatPage` mounts. + * + * One-shot semantics — `consumePendingMessage` returns and clears. + * If a second deep link arrives before consumption, the latest + * message wins (older drops with a Sentry breadcrumb). Renderer + * reloads / hard navigates blow this away because it's + * not persisted — by design, deep links are transient signals. + * + * @see {@link https://zustand.docs.pmnd.rs/} + */ + +import { create } from "zustand"; + +import { createSelectors } from "@/utils/create-selectors"; + +export interface PendingDeepLinkState { + /** Latest pending `deeplink.send` message text, or `null` if none. */ + pendingComposerMessage: string | null; +} + +export interface PendingDeepLinkActions { + /** + * Set the pending composer message. If one is already pending, + * it's overwritten — the most recent deep link wins. Used by the + * global consumer in `useGlobalDeepLinkConsumer`. + */ + setPendingComposerMessage: (message: string) => void; + /** + * Read and clear the pending composer message. Returns `null` if + * none was set. Used by `useDeepLinkConsumer` in the chat domain. + */ + consumePendingComposerMessage: () => string | null; +} + +export type PendingDeepLinkStore = PendingDeepLinkState & + PendingDeepLinkActions; + +const usePendingDeepLinkStoreBase = create()( + (set, get) => ({ + pendingComposerMessage: null, + setPendingComposerMessage: (message) => + set({ pendingComposerMessage: message }), + consumePendingComposerMessage: () => { + const message = get().pendingComposerMessage; + if (message !== null) set({ pendingComposerMessage: null }); + return message; + }, + }), +); + +export const usePendingDeepLinkStore = createSelectors( + usePendingDeepLinkStoreBase, +); + +/** + * Reset hook for tests. Not intended for production callers. + */ +export function __resetPendingDeepLinkForTesting(): void { + usePendingDeepLinkStoreBase.setState({ pendingComposerMessage: null }); +} From d84b5ef9aba71aa99407e5b3adf63e94fa8b49ab Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 30 May 2026 15:07:11 +0000 Subject: [PATCH 3/5] fix(macos): main-process deep-link path activates the window directly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex P2 — the renderer-side `ensureMainWindowVisible()` doesn't fire when no renderer exists, which is exactly the macOS no-window state (`window-all-closed` doesn't quit on Darwin; user closes the main window, app keeps running in the tray / accessory mode). Trace of the bug: 1. User closes main window. App alive, no renderer. 2. User clicks vellum://thread/abc. 3. `open-url` → `handleDeepLink` → parses, buffers (subscriberCount === 0), broadcasts (no windows to receive). 4. Window never recreates. Link sits in buffer indefinitely. 5. Renderer-side activation depended on a subscriber that doesn't exist. Fix: `handleDeepLink` calls `ensureVisible()` from main-window.ts for actionable kinds — independent of whether a renderer is alive. The renderer then mounts, drains the buffered link, and acts on it. The renderer-side `ensureMainWindowVisible()` call stays as defense-in-depth (no-op when already visible). `unknown` kinds explicitly skip activation — we don't want a foreign-scheme URL (`javascript:`, `data:`, etc.) routed by the OS to give an attacker a UI side effect. Tests added (4 new cases): - send activates the window - openThread activates the window - unknown / foreign schemes / malformed URLs do NOT activate - buffer-then-activate sequence: link is parked AND window recreates so the renderer-on-mount drain delivers it (the bug's primary fix path) 30/30 deep-links tests green. https://claude.ai/code/session_011sZ4p8AkqDQMSavHvWfioe --- apps/macos/src/main/deep-links.test.ts | 56 ++++++++++++++++++++++++++ apps/macos/src/main/deep-links.ts | 19 ++++++++- 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/apps/macos/src/main/deep-links.test.ts b/apps/macos/src/main/deep-links.test.ts index d20e511c4ca..f82568deacd 100644 --- a/apps/macos/src/main/deep-links.test.ts +++ b/apps/macos/src/main/deep-links.test.ts @@ -30,6 +30,15 @@ mock.module("electron", () => ({ 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,6 +60,7 @@ beforeEach(() => { setAsDefaultProtocolClientMock.mockClear(); ipcHandleMock.mockClear(); ipcOnMock.mockClear(); + ensureMainWindowVisibleMock.mockClear(); windows = []; }); @@ -342,3 +352,49 @@ 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("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" }]); + }); +}); diff --git a/apps/macos/src/main/deep-links.ts b/apps/macos/src/main/deep-links.ts index 2677287e42d..11275a3a918 100644 --- a/apps/macos/src/main/deep-links.ts +++ b/apps/macos/src/main/deep-links.ts @@ -1,5 +1,7 @@ import { BrowserWindow, app, ipcMain } from "electron"; +import { ensureVisible as ensureMainWindowVisible } from "./main-window"; + /** * Inbound deep links — `vellum://` and `vellum-assistant://` URL * schemes. The OS routes any user click on a `vellum://send?message=hi` @@ -131,14 +133,27 @@ 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. */ export const handleDeepLink = (input: string): void => { const link = parseVellumUrl(input); if (subscriberCount === 0) pending.push(link); broadcast(link); + if (link.kind !== "unknown") { + void ensureMainWindowVisible(); + } }; let installed = false; From 17b4a7df93c9213523d823af3c117f280957bd96 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 30 May 2026 15:35:52 +0000 Subject: [PATCH 4/5] docs(macos+web): scrub false breadcrumb claim + document cold/hot path activation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two non-blocking observations from review, both docs-only: 1. `pending-deep-link-store.ts` docstring claimed "older drops with a Sentry breadcrumb" on overwrite — but `setPendingComposerMessage` is a plain set with no breadcrumb. Drop the claim: silent overwrite ("latest wins") is honest and two-link-overwrite is below the noise floor. 2. `deep-links.ts` `handleDeepLink` now has a paragraph explaining the intentional duplicate activation between main (cold path — no-renderer activation) and renderer (`useGlobalDeepLinkConsumer`'s hot path — minimized / behind another window). The second call is a no-op via `ensureVisible`'s short-circuit; the comment saves the next reader the trace. No behavior changes. https://claude.ai/code/session_011sZ4p8AkqDQMSavHvWfioe --- apps/macos/src/main/deep-links.ts | 6 ++++++ apps/web/src/stores/pending-deep-link-store.ts | 11 ++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/apps/macos/src/main/deep-links.ts b/apps/macos/src/main/deep-links.ts index 11275a3a918..5d6b244b4bb 100644 --- a/apps/macos/src/main/deep-links.ts +++ b/apps/macos/src/main/deep-links.ts @@ -146,6 +146,12 @@ const broadcast = (link: DeepLink): void => { * 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); diff --git a/apps/web/src/stores/pending-deep-link-store.ts b/apps/web/src/stores/pending-deep-link-store.ts index 1a999280cfb..7a1432e82d7 100644 --- a/apps/web/src/stores/pending-deep-link-store.ts +++ b/apps/web/src/stores/pending-deep-link-store.ts @@ -11,11 +11,12 @@ * would be dropped — the bus event publishes to no chat-domain * subscriber until `ChatPage` mounts. * - * One-shot semantics — `consumePendingMessage` returns and clears. - * If a second deep link arrives before consumption, the latest - * message wins (older drops with a Sentry breadcrumb). Renderer - * reloads / hard navigates blow this away because it's - * not persisted — by design, deep links are transient signals. + * One-shot semantics — `consumePendingComposerMessage` returns and + * clears. If a second deep link arrives before consumption, the + * latest message wins (silent overwrite — two-link-overwrite is + * below the noise floor in practice). Renderer reloads / hard + * navigates blow this away because it's not persisted — by design, + * deep links are transient signals. * * @see {@link https://zustand.docs.pmnd.rs/} */ From 4f063cb1f230c9e2e980e56edcf10bf2aa096eb7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 30 May 2026 15:40:38 +0000 Subject: [PATCH 5/5] fix(macos): gate deep-link activation on app.isReady + track subscribers by WebContents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two new Codex P1s, both real, both closed by the same restructuring. ## Defer activation until Electron is ready Cold-launch trace: `will-finish-launching` → `open-url` fires BEFORE `app.whenReady()`. My previous `ensureMainWindowVisible()` call from `handleDeepLink` reaches `new BrowserWindow()` pre-ready, which races Electron's init. Fix: gate `ensureMainWindowVisible()` on `app.isReady()`. Pre-ready the link is already buffered above; the initial `installMainWindow` in the `whenReady` chain creates the first window which drains the link on mount. Post-ready the activation fires for the recreate case. ## Track subscribers by WebContents Previous counter-based model leaked: React effect cleanup is not guaranteed to run when a webContents is destroyed (window-close kills the JS context before useEffect cleanups flush), so the counter could stay positive with zero live subscribers → next link sees count > 0 → not buffered → broadcast to no listener → lost. Fix: `Set` keyed by `event.sender`, with a per-sender `once("destroyed", ...)` listener that self-cleans regardless of React's behavior. Common subscribe/unsubscribe path unchanged from the renderer's perspective; the destroyed listener is defense-in- depth. Closes LUM-2074 (subscriber-set + destroyed listener) by construction — was filed as a follow-up, now landing in this PR since the same Codex finding requires it. ## Tests - `defers activation when app is not yet ready (cold-launch via vellum://)`. - `activates after app becomes ready (warm path)`. - `destroyed webContents auto-clears its subscription (no leak when React cleanup misses)`. - Existing subscribe / unsubscribe / drain tests refactored to pass a synthetic `event.sender` with a `once("destroyed", ...)` capture helper (`makeSender()`). - `subscribe/unsubscribe IPC accounting is reference-counted and never goes negative` renamed to `unsubscribe with no matching subscriber is a no-op (idempotent delete)` — set-based semantics make the assertion clearer. 33/33 deep-links tests green; 9/9 macOS test files green. https://claude.ai/code/session_011sZ4p8AkqDQMSavHvWfioe --- apps/macos/src/main/deep-links.test.ts | 106 +++++++++++++++++++++---- apps/macos/src/main/deep-links.ts | 68 ++++++++++------ 2 files changed, 134 insertions(+), 40 deletions(-) diff --git a/apps/macos/src/main/deep-links.test.ts b/apps/macos/src/main/deep-links.test.ts index f82568deacd..aa42a62cc2f 100644 --- a/apps/macos/src/main/deep-links.test.ts +++ b/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) => + ipcOnListeners.get("vellum:deepLinks:subscribe")?.({ sender: s.sender }); +const unsubscribeWith = (s: ReturnType) => + ipcOnListeners.get("vellum:deepLinks:unsubscribe")?.({ sender: s.sender }); const appListeners = new Map(); const appOnMock = mock((event: string, listener: Listener) => { appListeners.set(event, listener); @@ -21,10 +43,12 @@ let windows: Array<{ webContents: { send: ReturnType }; }> = []; +let appIsReady = true; mock.module("electron", () => ({ app: { on: appOnMock, setAsDefaultProtocolClient: setAsDefaultProtocolClientMock, + isReady: () => appIsReady, }, ipcMain: { handle: ipcHandleMock, on: ipcOnMock }, BrowserWindow: { getAllWindows: () => windows }, @@ -62,6 +86,7 @@ beforeEach(() => { ipcOnMock.mockClear(); ensureMainWindowVisibleMock.mockClear(); windows = []; + appIsReady = true; }); afterEach(() => { @@ -242,7 +267,8 @@ 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. @@ -250,8 +276,9 @@ describe("installDeepLinks", () => { // 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([]); }); @@ -261,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([ @@ -297,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]; @@ -308,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", () => { @@ -372,6 +423,29 @@ describe("handleDeepLink — window activation", () => { 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 diff --git a/apps/macos/src/main/deep-links.ts b/apps/macos/src/main/deep-links.ts index 5d6b244b4bb..139d8d194ef 100644 --- a/apps/macos/src/main/deep-links.ts +++ b/apps/macos/src/main/deep-links.ts @@ -1,4 +1,9 @@ -import { BrowserWindow, app, ipcMain } from "electron"; +import { + BrowserWindow, + app, + ipcMain, + type WebContents, +} from "electron"; import { ensureVisible as ensureMainWindowVisible } from "./main-window"; @@ -104,18 +109,19 @@ 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 @@ -123,7 +129,7 @@ const pending: DeepLink[] = []; // `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(); const broadcast = (link: DeepLink): void => { for (const win of BrowserWindow.getAllWindows()) { @@ -155,9 +161,16 @@ const broadcast = (link: DeepLink): void => { */ 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); - if (link.kind !== "unknown") { + // 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(); } }; @@ -196,15 +209,22 @@ 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); }); }; @@ -212,6 +232,6 @@ export const installDeepLinks = (): void => { // uses `installDeepLinks` instead. export const __resetForTesting = (): void => { installed = false; - subscriberCount = 0; + subscribers.clear(); pending.length = 0; };