feat(macos+web): vellum:// deep-link bridge → event bus, with pre-mount buffering (LUM-1872)#32656
Conversation
…fering (LUM-1872)
## Capability
`vellum://send?message=…` and `vellum://thread/<id>` URLs routed by
Launch Services from any app (Mail, Slack, browser address bar,
`open vellum://...` shell) reach the running Electron app, parse
into typed `DeepLink`, and broadcast to the renderer where they fan
into typed bus events. Chat domain (separate PR) subscribes to
`deeplink.send` / `deeplink.openThread` to act.
## Shape — second use of the ELECTRON.md push-signals pattern,
extended with pre-renderer-ready buffering
Same 5-step shape as the power-events bridge (LUM-1974) — the
convention pays off on its first reuse. One addition: deep links
can arrive BEFORE the renderer exists (OS launches the app via a
URL click → `open-url` fires before `whenReady`). Captured in a
new bridge surface pair documented in ELECTRON.md.
1. `apps/macos/src/main/deep-links.ts` — `installDeepLinks()`:
- `app.setAsDefaultProtocolClient` for `vellum` + `vellum-assistant`
- `app.on("will-finish-launching")` → registers `open-url`
(the #1 deep-link bug: registering in `whenReady` misses the
launching URL)
- `parseVellumUrl(url)` pure parser → typed `DeepLink`
- `extractDeepLinkFromArgv(argv)` for Windows/Linux
second-instance forwarding
- Buffer + broadcast: pending links go into module-scope
`pending[]`, also broadcast via `webContents.send` to every
BrowserWindow
- `ipcMain.handle("vellum:deepLinks:drain")` returns + clears
the buffer
- Foreign-scheme rejection (`javascript:`, `data:`, `file:`,
`http:`) → `kind: "unknown"` so the bridge surface stays
exhaustive
2. `apps/macos/src/main/index.ts` — calls `installDeepLinks()` at
top-level (before `whenReady`) so the will-finish-launching
subscription captures launching URLs. Extends the existing
`second-instance` handler to also extract URLs from argv via
`extractDeepLinkFromArgv`, covering Windows/Linux where
`open-url` never fires.
3. `apps/macos/src/preload/index.ts` —
`window.vellum.deepLinks.drain() → Promise<DeepLink[]>` +
`onLink(callback) → unsubscribe`. `DeepLink` type mirrored
inline per the existing convention.
4. `apps/web/src/runtime/is-electron.ts` — ambient declaration.
5. `apps/web/src/runtime/deep-links.ts` —
`subscribeToDeepLinks(cb)` + `drainPendingDeepLinks()`. Both
no-op off Electron.
6. `apps/web/src/stores/event-bus-store.ts` — `BusEventMap` grows
`deeplink.send { message }`, `deeplink.openThread { threadId }`,
`deeplink.unknown { url }`.
7. `apps/web/src/hooks/use-event-bus-init.ts` —
subscribe-then-drain order matters: a link landing between drain
completion and subscription would be lost otherwise. Subscribe
first, drain second. Each drained or live link is narrowed into
the appropriate typed bus event.
8. `apps/web/docs/EVENT_BUS.md` + `apps/web/docs/ELECTRON.md` —
table grows the three `deeplink.*` events; ELECTRON.md gains a
sub-section on "when signals can arrive before the renderer
exists" documenting the subscribe-then-drain pattern.
## Tests
- `apps/macos/src/main/deep-links.test.ts` (22 cases): parser
matrix (send happy/empty/decoded, openThread happy/multi-segment/
missing-id, foreign schemes javascript/file/http, malformed URLs,
unrecognized hosts); argv extraction (matches scheme, alternate
scheme, returns null); install (both schemes registered +
idempotent, will-finish-launching → open-url chain, open-url
preventDefault + buffer, drain IPC clears buffer); broadcast (all
windows, skips destroyed, unknown-kind still broadcast).
- `apps/web/src/hooks/use-event-bus-init.test.tsx` (+5 cases):
live `deeplink.send` / `deeplink.openThread` / `deeplink.unknown`
publish to typed bus events; drained pending links publish in
order; subscribe-BEFORE-drain ordering verified via
`invocationCallOrder`; unsubscribe on unmount stops live delivery.
53 renderer bus tests + 9 macOS test files green.
## Out of scope (follow-up PRs)
- **Renderer-side actions**: composer pre-fill (`deeplink.send`)
and thread navigation (`deeplink.openThread`) — owned by the chat
domain, separate ticket.
- **electron-builder `protocols` Info.plist block** — the build
pipeline doesn't ship a .app yet (LUM-2024 territory). Dynamic
`setAsDefaultProtocolClient` works for dev today; the static
registration matters only for distribution.
- **Bringing window forward + accessory-mode transition** on deep
link: the bridge currently broadcasts but doesn't focus the
window. The chat domain's deeplink consumer should call
`ensureMainWindowVisible` (via a new bridge method or by
triggering an existing one) when it acts. Captured here so the
signal-source PR isn't overloaded with consumer concerns.
https://claude.ai/code/session_011sZ4p8AkqDQMSavHvWfioe
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef548b4ba7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| */ | ||
| export const handleDeepLink = (input: string): void => { | ||
| const link = parseVellumUrl(input); | ||
| pending.push(link); |
There was a problem hiding this comment.
Avoid replaying live deep links from the drain buffer
When the renderer is already mounted, handleDeepLink still pushes every incoming URL into the global pending buffer before broadcasting it. That live link is handled immediately through vellum:deepLinks:event, but it remains in pending until the next drain() call, so a renderer reload/remount or another renderer that drains later will publish the same deeplink.send/deeplink.openThread a second time. This can duplicate user-visible actions such as sending/prefilling a message or navigating to a thread; the buffer needs to distinguish pre-renderer backlog from live events that have already been delivered.
Useful? React with 👍 / 👎.
…r reload Codex P2 — the prior implementation pushed EVERY incoming link into `pending[]` AND broadcast it. Live links arriving after the renderer had mounted were handled immediately via the broadcast channel BUT stayed in the buffer. A renderer hard-navigate (e.g. logout) followed by drain on the new renderer would replay already-handled links — composer re-pre-fills with a stale message, navigator re-opens a stale thread, user-visible double-action. Fix: a `drained` flag flips on the first drain call. Before drain, buffer + broadcast (broadcast is wasted, no listeners yet; buffer is what matters). After drain, broadcast only — no buffering, nothing to replay. Caveat called out in the inline doc: a deep link arriving in the narrow window between a renderer hard-navigate and the new renderer's drain is lost. That's the accepted tradeoff — duplicate-action is worse than during-reload lost-link (rare in this app, recoverable by re-clicking). Tests added: - post-drain live links do NOT enter the buffer (renderer-reload scenario: drain backlog, fire a live link, simulate hard navigate, second drain returns empty). - post-drain live links still broadcast (live subscribers still receive them — only the buffer is suppressed, not the broadcast). 24/24 deep-links tests green. https://claude.ai/code/session_011sZ4p8AkqDQMSavHvWfioe
|
Codex P2 (live links replayed on renderer reload) addressed in fbdc7be. Fix: a Caveat called out in the inline doc: a deep link arriving in the narrow window between a renderer hard-navigate (e.g. logout) and the new renderer's drain is lost. That's the accepted tradeoff — duplicate-action is worse than during-reload lost-link, which is rare in this app and recoverable by re-clicking. Two tests added:
24/24 deep-links tests green. Generated by Claude Code |
Audit-found bug. The `drained` flag was permanent — once flipped on
first drain, links arriving while NO renderer is subscribed (logout,
between hard navigates) bypass the buffer because broadcast hits zero
listeners and the post-drained "live mode" suppresses buffering.
Real-world trace:
1. User logged out, clicks vellum://send?message=hello → buffered. ✓
2. User logs in → drain returns it. drained = true forever. ✓
3. User logs out → renderer unmounts.
4. User clicks vellum://thread/abc → broadcast hits zero listeners,
drained = true so no buffer → LINK LOST.
5. User logs back in → drain returns []. Thread never opens.
The flag conflated "has ever drained" with "is subscribed right now."
Fix: explicit subscriber tracking via two new IPC channels:
- `vellum:deepLinks:subscribe` — preload sends on `onLink`
registration. Main increments `subscriberCount`.
- `vellum:deepLinks:unsubscribe` — preload sends on cleanup.
Main decrements (clamped to 0).
`handleDeepLink` buffers iff `subscriberCount === 0`. `drain` always
returns + clears the buffer; no more permanent flag.
Closes the logout-relogin lost-link case AND keeps the Codex P2 fix
(live links never enter the buffer when a subscriber is listening,
so they can't be replayed on renderer reload).
Residual race (called out in inline doc): a link arriving in the
sub-microsecond window between renderer's `ipcRenderer.on`
registration and main's processing of the `subscribe` IPC could be
buffered + broadcast. Not realistically triggerable by user action
on the same event loop.
Tests:
- with-subscriber: live links broadcast but don't buffer (Codex P2
stays closed) — verified by subscribe, fire live link,
unsubscribe+resubscribe, assert drain is empty.
- logout-relogin: link arriving while unsubscribed lands in buffer
for the next subscriber (the new bug fix) — subscribe, drain,
unsubscribe, fire link, resubscribe, assert drain returns it.
- Unsubscribe-before-subscribe clamps to 0 (no negative reference
count if cleanup order goes weird).
- Live broadcast still reaches subscribed renderers.
26/26 deep-links tests + 34/34 bus-init tests green.
https://claude.ai/code/session_011sZ4p8AkqDQMSavHvWfioe
Audit-found bug + fix —
|
There was a problem hiding this comment.
✦ APPROVE — reviewed at bf959ca7ae
Value: Second use of the push-signal bridge pattern from #32651, extended with the new "signals can arrive before the renderer exists" sub-pattern (subscribe-then-drain + main-side buffering). The convention proves out on its first reuse — and the pre-mount buffering layer is exactly the right place to land it, since deep links are the canonical case where the OS hands you a signal before your UI exists.
What this does:
- Main:
parseVellumUrl(pure, exhaustive over send/openThread/unknown),extractDeepLinkFromArgv(Windows/Linux second-instance),installDeepLinkswires both schemes +will-finish-launching → open-url(the launching-URL trap) +second-instance+vellum:deepLinks:drainIPC + newsubscribe/unsubscribeIPC channels. - Bridge:
window.vellum.deepLinks.drain()+onLink(cb)with subscribe/unsubscribe IPC sent inside the callback lifecycle. - Renderer:
subscribeToDeepLinks+drainPendingDeepLinksruntime wrappers (no-op off Electron), three newBusEventMapentries (deeplink.send/deeplink.openThread/deeplink.unknown),use-event-bus-initsubscribes first then drains. - Docs:
ELECTRON.mdgains the "signals before renderer exists" sub-section;EVENT_BUS.mdevent table extended.
Full analysis
Codex P2 (live-link replay on renderer reload) — closed at fbdc7bef51, refined at bf959ca7ae
The original drained: boolean flag closed the replay case but introduced the logout-relogin gap that your audit caught. The pivot to subscriber-counted buffering is the right shape — it stops conflating "has drained" with "is subscribed now":
if (subscriberCount === 0) pending.push(link);
broadcast(link);This closes both cases by construction:
- Renderer reload, subscriber present: count > 0 → broadcast-only → drain returns nothing the second time.
- Logout, no subscriber: count === 0 → buffered → next renderer drains it.
Test coverage matches the intent: "with a subscriber present, live links broadcast but do NOT enter the buffer (no replay on renderer reload)" + "logout-relogin: link arriving while unsubscribed lands in the buffer for the next subscriber" + the clamp-to-zero accounting test. The pairing of test names to failure modes is exemplary — anyone reading the test list sees the model.
Residual race — documented + accepted
The inline comment on subscriberCount calls out the sub-microsecond window between renderer's ipcRenderer.on and main's processing of the subscribe IPC. Acknowledging it inline is the right call — it's not realistically triggerable by user action (deep links arrive on user click, not at IPC-roundtrip frequency), but the comment prevents the next reader from "fixing" it with a more complex design.
Subscribe-then-drain ordering
use-event-bus-init.ts:lines ~150–165 does the subscribe-first, drain-second sequence. Verified the comment explanation matches the implementation — the inline note about "any in-flight link is delivered via onLink and the drained buffer carries the pre-renderer-ready backlog" is correct as long as the main-side buffer is subscriber-counted (which it is at HEAD).
Pattern extension — ELECTRON.md
The new sub-section codifies the rule: bridges where the signal can arrive pre-mount need both a subscribe<X> (live) and a drainPending<X> (backlog) surface, and consumers must subscribe-then-drain. This is the kind of documentation that prevents the next contributor from rediscovering the race by hand. High-leverage.
Observations (non-blocking)
-
Renderer crash / freeze leaks subscriberCount. If a
webContentscrashes without the preload cleanup running (which is the cleanup path forsubscribe → unsubscribeIPC), the count stays incremented. Subsequent links arrive with count > 0 → broadcast-only → lost (no live consumer to receive them). A more robust shape: tracksubscribers: Set<WebContents>on the firstsubscribeIPC from each sender and listen forwebContents.on("destroyed")to clean up. Same semantics for the common path; survives crashes. Doesn't need to land in this PR — it's a refinement of a residual edge case, not a correctness gap on the common path. -
Unhandled rejection in
drainPendingDeepLinks().then(...). The.thenhas no.catch. IfpublishDeepLink(orbus.publishdownstream) throws, the rejection is silent. TheappStateChangeimport in the same effect has aSentry.captureException.catch— worth mirroring here for parity:void drainPendingDeepLinks() .then((pending) => { for (const link of pending) publishDeepLink(link); }) .catch((err) => Sentry.captureException(err, { level: "warning", tags: { context: "deep_link_drain" } }));
-
use-event-bus-init.tssize. Confirming your own audit-comment flag — at ~290 lines and growing one effect-branch per source, the "one helper per signal source" refactor is overdue. Right call to leave it out of this PR. When you do it, the test file colocation should follow the same split (today it's one mega-file).
Merge gate
- CI 11/11 green at HEAD
- Codex review at HEAD: no comments (P2 closed in code)
- Devin review: 0 potential issues
- 26 deep-links tests + 5 bus-init tests covering both replay + logout-relogin regressions
Mergeable.
Vellum Constitution — Conventions pay off on reuse: the push-signal bridge pattern from #32651 ports cleanly to deep links and extends to the pre-renderer case without bending. The subscriber-counted buffering refinement is the kind of audit-driven correction that turns a working fix into a structurally correct one.
Picks up non-blocking observation from review. The drain promise had no `.catch`, so a downstream `publishDeepLink` / `bus.publish` throw would surface as an unhandled rejection. Mirrors the `appStateChange` Sentry-capture pattern already in the same effect. https://claude.ai/code/session_011sZ4p8AkqDQMSavHvWfioe
|
Vex non-blocking observations triaged:
34/34 bus-init tests still green after the Generated by Claude Code |
There was a problem hiding this comment.
✦ Re-APPROVE at e6641a6802 — picks up Observation #2 from prior review (Sentry-capture the drain rejection). Exactly the suggested shape, mirroring the appStateChange pattern in the same effect. CI 11/11 green. Mergeable.
Summary
vellum://send?message=…andvellum://thread/<id>URLs routed by Launch Services from any app (Mail, Slack, browser,open vellum://…shell) reach the running Electron app, parse into typedDeepLink, and broadcast to the renderer where they fan into typed bus events. The chat domain (separate PR) subscribes todeeplink.send/deeplink.openThreadto act.Shape — second use of the ELECTRON.md push-signals pattern, extended with pre-renderer-ready buffering
Same 5-step shape as the power-events bridge (#32651) — the convention pays off on its first reuse. One addition: deep links can arrive BEFORE the renderer exists (OS launches the app via a URL click →
open-urlfires beforewhenReady). Captured in a new bridge surface pair documented inELECTRON.md.Main process
apps/macos/src/main/deep-links.ts—installDeepLinks():app.setAsDefaultProtocolClientforvellum+vellum-assistant.app.on("will-finish-launching")→ registersopen-url(the feat: initialize Next.js app in /web directory #1 deep-link bug: registering inwhenReadymisses the launching URL).parseVellumUrl(url)pure parser → typedDeepLinkdiscriminated union.extractDeepLinkFromArgv(argv)for Windows / Linux second-instance forwarding (macOS uses a freshopen-url; the others use argv).pending[], also broadcast viawebContents.sendto every BrowserWindow.ipcMain.handle("vellum:deepLinks:drain")returns + clears the buffer.javascript:,data:,file:,http:) →kind: "unknown"so the bridge surface stays exhaustive.apps/macos/src/main/index.ts— callsinstallDeepLinks()at top-level (beforewhenReady) so thewill-finish-launchingsubscription captures launching URLs. Extends the existingsecond-instancehandler to also extract URLs from argv.Bridge + renderer
window.vellum.deepLinks.drain() → Promise<DeepLink[]>+onLink(callback) → unsubscribe.DeepLinktype mirrored inline per existing convention.subscribeToDeepLinks(cb)+drainPendingDeepLinks(). Both no-op off Electron.BusEventMapgrowsdeeplink.send { message },deeplink.openThread { threadId },deeplink.unknown { url }.use-event-bus-init— subscribe-then-drain order matters: a link landing between drain completion and subscription would be lost otherwise. Subscribe first, drain second. Each link narrows into the appropriate typed bus event.Docs
EVENT_BUS.md— event table grows the threedeeplink.*entries.ELECTRON.md— new sub-section "When signals can arrive before the renderer exists" documenting the subscribe-then-drain pattern (subscribe<X>for live +drainPending<X>for the main-side buffer).Tests
apps/macos/src/main/deep-links.test.ts(22 cases):will-finish-launching → open-urlchain,open-urlpreventDefault + buffer, drain IPC clears buffer.apps/web/src/hooks/use-event-bus-init.test.tsx(+5 cases):deeplink.send/deeplink.openThread/deeplink.unknownpublish to typed bus events.invocationCallOrder.Out of scope (follow-up PRs)
deeplink.send) and thread navigation (deeplink.openThread) — owned by the chat domain, separate ticket.protocolsInfo.plist block — the build pipeline doesn't ship a .app yet (LUM-2024 territory). DynamicsetAsDefaultProtocolClientworks for dev today; the static registration matters only for distribution.ensureMainWindowVisiblewhen it acts. Captured here so the signal-source PR isn't overloaded with consumer concerns.Test plan
bun --cwd apps/macos run typecheck— green.bun --cwd apps/macos run test:ci— 9 test files, all green (incl. new 22-casedeep-links.test.ts).bun --cwd apps/macos run build— main + preload bundles build cleanly.bun --cwd apps/web run typecheck— green for files this PR touches.bun --cwd apps/web test src/hooks/use-event-bus-init.test.tsx src/stores/event-bus-store.test.ts— 53 cases green.open vellum://send?message=hellofrom another app while Electron dev is running → renderer logs the parsed event.open vellum://thread/abc→ same, withopenThread.open vellum://garbage→deeplink.unknownpublished.https://claude.ai/code/session_011sZ4p8AkqDQMSavHvWfioe
Generated by Claude Code