fix(desktop): route t3code:// thread deep links instead of only raising the window - #7994
fix(desktop): route t3code:// thread deep links instead of only raising the window#7994CurlyPeter wants to merge 1 commit into
Conversation
…ng the window The app registers the t3code:// scheme (Info.plist on macOS, the Clerk bridge's setAsDefaultProtocolClient, the Linux .desktop entry) but routes exactly one URL family: Clerk's OAuth callback. Every other URL is accepted by the OS, delivered to the app and dropped, so opening a thread link brings the window to the front and leaves it wherever it already was. The renderer has had the route all along (/$environmentId/$threadId) and nothing connects the two. This accepts the shape the relay already emits for its push notifications (deepLink: "/threads/<environmentId>/<threadId>") and the mobile router already matches, so one link opens the same chat on a phone and on a desktop. Delivery reuses the existing menu-action channel rather than adding one: dispatchMenuAction already creates the window when there is none, defers the send until did-finish-load while the renderer boots, and reveals the window afterwards. A dedicated channel would have duplicated all of it plus a preload entry. The renderer rule lives in its own module so the decision is testable without rendering and AppSidebarLayout keeps a single call. The cold-start path needed three supporting fixes, because the launch is precisely when nothing is ready yet: - open-url is captured synchronously at process bootstrap. Electron buffers nothing and documents that a listener registered in response to `ready` misses the URL that launched the app; even registering before whenReady sits behind the whole layer graph plus the shell environment, user-data path, settings load and Clerk bootstrap, each of which yields to the run loop. - The preload buffers menu actions until the renderer subscribes. onMenuAction attaches its IPC listener lazily and its only caller renders inside the authenticated app shell, which is gated on an async bootstrap, while the main process sends on did-finish-load, which does not wait for it. On a cold start the action was otherwise sent into a channel with no listener. - DesktopWindow parks a menu action that arrives with no window and no ready backend, and replays it from handleBackendReady through the same delivery path. That state is invisible for a menu item, since the user can click again, and terminal for a launch URL. The drain sits in `ensuring` so a failed window creation cannot strand it. Parsing is deliberately at least as strict as the mobile app's normalizeThreadDeepLink: no query, no fragment, no credentials, exact segment count, and both ids must be UUIDs. The UUID requirement is a boundary rather than a format preference, because a URL handler is an unauthenticated entry point: without it t3code://threads/settings/connections resolves to /settings/connections, a real static route that outranks the thread route, so any local process could drive the window onto a settings page. The listener declines quietly for anything that is not a thread link and calls preventDefault only for URLs it owns, so Clerk's callbacks are untouched. open-url is macOS-only; Windows and Linux deliver the URL as argv to a second instance, and wiring that path is left for a follow-up rather than guessed at.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| // The preload runs before any page script, so a listener installed here cannot | ||
| // miss anything. | ||
| const pendingMenuActions: string[] = []; | ||
| ipcRenderer.on(IpcChannels.MENU_ACTION_CHANNEL, (_event, action: unknown) => { |
There was a problem hiding this comment.
🟠 High src/preload.ts:44
Buffered MENU_ACTION_CHANNEL actions are never delivered to the onMenuAction subscriber, and every action remains in pendingMenuActions indefinitely. onMenuAction only registers a separate IPC listener and never drains the array, so the cold-start deep-link action is lost while later actions leak memory. Drain the buffer when subscribing and remove each action after delivery.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/desktop/src/preload.ts around line 44:
Buffered `MENU_ACTION_CHANNEL` actions are never delivered to the `onMenuAction` subscriber, and every action remains in `pendingMenuActions` indefinitely. `onMenuAction` only registers a separate IPC listener and never drains the array, so the cold-start deep-link action is lost while later actions leak memory. Drain the buffer when subscribing and remove each action after delivery.
| yield* createMainIfBackendReady.pipe( | ||
| Effect.ensuring( | ||
| Effect.gen(function* () { | ||
| const pending = yield* Ref.getAndSet(pendingMenuActionRef, Option.none()); |
There was a problem hiding this comment.
🟠 High window/DesktopWindow.ts:912
handleBackendReady permanently discards a pending deep link when deliverMenuAction fails, so a later successful window creation cannot replay it. Ref.getAndSet clears the slot before delivery and the caught failure prevents recovery; read the slot first and clear it only after delivery succeeds.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/desktop/src/window/DesktopWindow.ts around line 912:
`handleBackendReady` permanently discards a pending deep link when `deliverMenuAction` fails, so a later successful window creation cannot replay it. `Ref.getAndSet` clears the slot before delivery and the caught failure prevents recovery; read the slot first and clear it only after delivery succeeds.
There was a problem hiding this comment.
Effect service conventions: one finding — the launch-URL buffer that DesktopUrlRouting.register consumes lives in module globals instead of the Effect environment. Everything else in the new service (namespace imports, inline Context.Service interface, make/layer exports, runPromiseWith at the Electron callback boundary) matches the existing desktop services.
Posted via Macroscope — Effect Service Conventions
| const launchUrls: string[] = []; | ||
| let stopCapturingLaunchUrls: (() => void) | null = null; | ||
|
|
||
| /** | ||
| * Start collecting `open-url` events immediately. Call once, synchronously, | ||
| * from the process entrypoint before any async work happens. | ||
| * | ||
| * Takes the app rather than importing electron, so this module stays importable | ||
| * in tests without an Electron runtime. | ||
| */ | ||
| export function captureLaunchUrlsSync(app: Electron.App): void { | ||
| if (stopCapturingLaunchUrls !== null) return; | ||
| const listener = (_event: Electron.Event, url: string) => { | ||
| launchUrls.push(url); | ||
| }; | ||
| app.on("open-url", listener); | ||
| stopCapturingLaunchUrls = () => app.removeListener("open-url", listener); | ||
| } |
There was a problem hiding this comment.
register drains launchUrls and flips stopCapturingLaunchUrls, so a real input of this service is hidden in module scope: it does not appear in make's requirements, and it is shared by every layer build in the process (a second runtime, or two tests in the same file, would see each other's buffer). The pre-ready capture genuinely has to run before the runtime exists, but the buffer itself can still be a value.
Consider having captureLaunchUrlsSync return a collector ({ readonly drain: () => ReadonlyArray<string>; readonly stop: () => void }), providing it from main.ts with Layer.succeed(DesktopLaunchUrls, collector), and acquiring it in make with yield* DesktopLaunchUrls.DesktopLaunchUrls. The dependency then shows up in the layer types and the drain/hand-over path becomes testable without an Electron runtime.
Posted via Macroscope — Effect Service Conventions
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 096cf1d. Configure here.
| ipcRenderer.on(IpcChannels.MENU_ACTION_CHANNEL, (_event, action: unknown) => { | ||
| if (typeof action === "string") pendingMenuActions.push(action); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Preload buffer never drained
High Severity
pendingMenuActions is filled by the bootstrap MENU_ACTION listener, but onMenuAction never replays or clears that buffer when the renderer finally subscribes. Cold-start deep links are sent on did-finish-load before AppSidebarLayout mounts (auth is still resolving), so they sit in the array forever and the chat never opens. The bootstrap listener also keeps pushing after subscribe, so the queue grows without bound.
Reviewed by Cursor Bugbot for commit 096cf1d. Configure here.
ApprovabilityVerdict: Skipped Macroscope did not run approvability analysis for this PR. Macroscope could not determine whether this PR modifies its approvability configuration, so the PR was not approved automatically. A PR that may change the rules that govern approval is never approved automatically. Not approved because:
|


The bug
The desktop app registers the
t3code://scheme (Info.pliston macOS, the Clerk bridge'ssetAsDefaultProtocolClient, the Linux.desktopentry) but routes exactly one URL family: Clerk's OAuth callback. Every other URL is accepted by the OS, delivered to the app and dropped.So opening a thread link brings the window to the front and leaves it wherever it already was. Measured:
open "t3code://app/<env>/<thread>"moves frontmost to T3 Code and nothing else happens; the onlyopen-urllistener in a shipped build belongs to@clerk/electron's OAuth transport, and there is no URL routing inapps/desktop/src.Meanwhile the renderer has had the route all along,
_chat.$environmentId.$threadId. Nothing connected the two.What this does
Accepts the shape the relay already emits for push notifications (
deepLink: "/threads/<environmentId>/<threadId>") and the mobile router already matches (threads/:environmentId/:threadId), so one link opens the same chat on a phone and on a desktop.Delivery reuses the existing menu-action channel rather than adding one.
dispatchMenuActionalready creates the window when there is none, defers the send untildid-finish-loadwhile the renderer boots, and reveals the window afterwards; a dedicated channel would have duplicated all of that plus a preload entry. The renderer rule lives in its own module so it is testable without rendering, andAppSidebarLayoutkeeps a single call.Cold start needed three supporting fixes
The launch is exactly the moment when nothing is ready, and each of these is invisible in the steady state:
open-urlis captured synchronously at process bootstrap. Electron buffers nothing and documents that a listener registered in response toreadymisses the URL that launched the app. Even registering beforewhenReadysits behind the whole layer graph plus the shell environment, user-data path, settings load and Clerk bootstrap, each of which yields to the run loop.onMenuActionattaches its IPC listener lazily, and its only caller renders inside the authenticated app shell, which is gated on an async bootstrap — while the main process sends ondid-finish-load, which does not wait for it. Without the buffer a cold-start action is sent into a channel with no listener.DesktopWindowparks a menu action that arrives with no window and no ready backend, replaying it fromhandleBackendReadythrough the same delivery path. That state is invisible for a menu item, since the user can click again, and terminal for a launch URL. The drain sits inensuringso a failed window creation cannot strand it.Parsing
Deliberately at least as strict as the mobile app's
normalizeThreadDeepLink: no query, no fragment, no credentials, exact segment count, and both ids must be UUIDs.The UUID requirement is a boundary, not a formatting preference. A URL handler is an unauthenticated entry point that any local process can drive. Without it,
t3code://threads/settings/connectionsresolves to/settings/connections— a real static route that outranks/$environmentId/$threadId— so anything on the machine could silently move the window onto a settings page.The listener declines quietly for anything that is not a thread link and calls
preventDefault()only for URLs it owns, so Clerk's callbacks are untouched.Scope
open-urlis macOS-only. Windows and Linux deliver the URL as argv to a second instance; that path is left for a follow-up rather than guessed at.Testing
@t3tools/desktop599 tests,@t3tools/web2737 tests. Both typechecks clean.desktop.urlRouting.register→handleUrl→dispatchMenuAction→deliverMenuAction, with the register span reportingbuffered: 1, i.e. the launch URL came through the bootstrap collector.Note
Medium Risk
Touches an unauthenticated OS protocol handler and in-app navigation. Parsing is UUID-strict and the renderer rejects non-absolute paths, but delivery still rides the shared menu-action IPC path.
Overview
Opens the same thread a
t3code://threads/<environmentId>/<threadId>link already opens on mobile, instead of only bringing the desktop window forward.Main process parses that shape (UUID ids only, no query/fragment/credentials, both authority forms) and sends a
navigate:action on the existing menu-action channel. Clerk OAuth URLs on the same scheme are left alone. Registration happens beforewhenReady, with a syncopen-urlcollector at process start so a launch URL is not missed. Windows/Linux argv forwarding is out of scope.Cold start is covered by parking a menu action until the backend is ready (replayed once from
handleBackendReady) and by buffering menu-action IPC in preload until the renderer can subscribe. The renderer mapsnavigate:to an in-app path and refuses protocol-relative or off-app targets.Reviewed by Cursor Bugbot for commit 096cf1d. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Route
t3code://thread deep links to the renderer instead of only raising the windowDesktopUrlRoutingservice that parses and validatest3code://threads/<environmentId>/<threadId>deep links, then dispatches anavigate:-prefixed menu action to the rendereropen-urlbootstrap listener in main.ts before Electron'swhenReadyso macOS cold-start events are not lostnavigate:actions in menuActionNavigation.ts and performs in-app navigation, rejecting external or protocol-relative targetsparseThreadDeepLinkin DesktopUrlRouting.ts only accepts UUID-shaped IDs and two authority forms (//threads/...and///threads/...); URLs with credentials, query, fragment, or extra segments are silently dropped📊 Macroscope summarized 096cf1d. 7 files reviewed, 2 issues evaluated, 0 issues filtered, 2 comments posted
🗂️ Filtered Issues