feat(desktop): register t3:// OS protocol handler for thread deep links#2424
feat(desktop): register t3:// OS protocol handler for thread deep links#2424davidmashburn wants to merge 1 commit into
Conversation
|
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 Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
github-actions
Bot
added
size:M
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review 5 blocking correctness issues found. This PR introduces a new OS protocol handler feature with multiple unresolved review comments identifying bugs in the deep link handling logic, including race conditions where links are dropped during cold start and issues with duplicate/stale thread opens. You can customize Macroscope's approvability policy. Learn more. |
|
Trying to integrate with the T3 code app and deep linking (the ability to focus specific chats programmatically specifically) would be a big help. |
Port thread deep-link handling into the Effect-based desktop stack so t3://thread/<id> opens the right conversation. Queue renderer navigation until the active environment is ready, route Alpha vs Nightly via distinct bundle IDs, and disable electron-builder publish auto-detection when GH_TOKEN is set so local artifact builds complete reliably. Co-authored-by: Cursor <cursoragent@cursor.com>
c1e8393 to
75a2103
Compare
|
Branch update: rebased onto current No server/orchestration changes in this revision — those are split out to #3642. |
github-actions
Bot
added
size:L
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 5 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 75a2103. Configure here.
| return () => { | ||
| unsubscribe?.(); | ||
| }; | ||
| }, [navigate, activeEnvironmentId, serverConfig]); |
There was a problem hiding this comment.
Deep link IPC before listener
High Severity
The onOpenThread listener in AppSidebarLayout registers after authentication, which means it can miss desktop:open-thread events sent early by the main process on did-finish-load. As IPC messages aren't replayed, deep links, especially during cold-starts or pre-auth, may be dropped.
Reviewed by Cursor Bugbot for commit 75a2103. Configure here.
| void navigate({ | ||
| to: "/$environmentId/$threadId", | ||
| params: { environmentId, threadId }, | ||
| }); |
There was a problem hiding this comment.
Deep link ignores thread environment
Medium Severity
onOpenThread navigates using only the active or primary environment id, not the environment that owns the thread. The codebase already exposes findThreadRef(threadId) for cross-environment lookup, but the new handler never uses it, so links to threads in another environment can land on / after the thread route check fails.
Reviewed by Cursor Bugbot for commit 75a2103. Configure here.
| } | ||
|
|
||
| sendOpenThreadToWindow(existingWindow.value, threadId); | ||
| }), |
There was a problem hiding this comment.
Stale pending thread overwrites newer
Medium Severity
When the first deep link sets pendingOpenThreadIdRef before a main window exists, a later openThread for a different id on an existing (still loading) window does not clear that pending value. The next did-finish-load flush still emits the older thread id, so navigation can jump to the wrong chat after a newer link.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 75a2103. Configure here.
|
|
||
| if (targetWindow.webContents.isLoadingMainFrame()) { | ||
| targetWindow.webContents.once("did-finish-load", send); | ||
| return; |
There was a problem hiding this comment.
Duplicate IPC while page loading
Medium Severity
Each openThread call while the main frame is loading adds another webContents.once("did-finish-load") sender without canceling prior ones. Repeated deep links (or duplicate macOS delivery) before load finishes can fire multiple desktop:open-thread messages for the same or different threads.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 75a2103. Configure here.
| const launchThreadId = findThreadDeepLinkInArgv(process.argv); | ||
| if (Option.isSome(launchThreadId)) { | ||
| yield* openThreadDeepLink(launchThreadId.value); | ||
| } |
There was a problem hiding this comment.
macOS cold start duplicate opens
Medium Severity
On macOS, launching via t3://thread/… can invoke openThreadDeepLink twice: once from findThreadDeepLinkInArgv(process.argv) in configure and again from the open-url handler. That can emit duplicate desktop:open-thread IPC and navigation for a single link.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 75a2103. Configure here.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟡 Medium
t3code/scripts/build-desktop-artifact.ts
Line 1314 in 75a2103
resolveDesktopBuildAppId now returns distinct appId values for the alpha and nightly channels, so both builds can be installed side-by-side. However, both channels still share the same Electron userData path (.../.t3/userdata/t3code), so they read and write the same profile, settings, and update-metadata files. Running both builds simultaneously causes them to overwrite each other's persisted state and auto-update data. Consider making the userDataDirName (or the stateDir) channel-specific — e.g. t3code-nightly for nightly builds — so each channel gets an isolated data directory.
🤖 Copy this AI Prompt to have your agent fix this:
In file @scripts/build-desktop-artifact.ts around line 1314:
`resolveDesktopBuildAppId` now returns distinct `appId` values for the `alpha` and `nightly` channels, so both builds can be installed side-by-side. However, both channels still share the same Electron `userData` path (`.../.t3/userdata/t3code`), so they read and write the same profile, settings, and update-metadata files. Running both builds simultaneously causes them to overwrite each other's persisted state and auto-update data. Consider making the `userDataDirName` (or the `stateDir`) channel-specific — e.g. `t3code-nightly` for nightly builds — so each channel gets an isolated data directory.
| void runPromise(handleDeepLinkUrl(url)); | ||
| }); | ||
|
|
||
| yield* electronApp.on( |
There was a problem hiding this comment.
🟡 Medium app/DesktopDeepLinks.ts:78
The second-instance handler is registered in registerEarly before the single-instance lock is acquired, so a t3:// link opened during startup on Windows/Linux launches a second app process instead of routing the URL to the starting instance. registerEarly attaches the second-instance listener early, but the lock (requestSingleInstanceLock) is only taken later during configure, leaving a window where the second-instance event is never emitted and the deep link is lost. Consider registering the second-instance listener only after the single-instance lock has been acquired, or document the startup ordering if this gap is intentional.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/desktop/src/app/DesktopDeepLinks.ts around line 78:
The `second-instance` handler is registered in `registerEarly` before the single-instance lock is acquired, so a `t3://` link opened during startup on Windows/Linux launches a second app process instead of routing the URL to the starting instance. `registerEarly` attaches the `second-instance` listener early, but the lock (`requestSingleInstanceLock`) is only taken later during `configure`, leaving a window where the second-instance event is never emitted and the deep link is lost. Consider registering the `second-instance` listener only after the single-instance lock has been acquired, or document the startup ordering if this gap is intentional.
|
|
||
| yield* appIdentity.configure; | ||
| yield* lifecycle.register; | ||
| yield* deepLinks.registerEarly; |
There was a problem hiding this comment.
🟡 Medium app/DesktopApp.ts:243
deepLinks.registerEarly is called after shellEnvironment.installIntoProcess, resolveUserDataPath, electronApp.setPath, desktopSettings.load, appIdentity.configure, and lifecycle.register. On a macOS cold start, Electron emits the open-url event very early — before this point — so a t3://... launch that triggers the initial open-url is dropped and the app opens without navigating to the deep link. Consider registering the open-url listener before these async initialization steps.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/desktop/src/app/DesktopApp.ts around line 243:
`deepLinks.registerEarly` is called after `shellEnvironment.installIntoProcess`, `resolveUserDataPath`, `electronApp.setPath`, `desktopSettings.load`, `appIdentity.configure`, and `lifecycle.register`. On a macOS cold start, Electron emits the `open-url` event very early — before this point — so a `t3://...` launch that triggers the initial `open-url` is dropped and the app opens without navigating to the deep link. Consider registering the `open-url` listener before these async initialization steps.
| }); | ||
| } | ||
|
|
||
| const launchThreadId = findThreadDeepLinkInArgv(process.argv); |
There was a problem hiding this comment.
🟡 Medium app/DesktopDeepLinks.ts:106
On startup, configure re-parses process.argv and treats any preserved t3://... argument as a fresh deep link. The app's own relaunch path restarts packaged builds with args: process.argv.slice(1), so if the app was originally launched from a t3:// deep link, subsequent relaunches (for example WSL/server-exposure changes) carry the stale URL forward and reopen that old thread unexpectedly. Consider checking only argv that was not carried over from a prior launch (for example the OS-provided launch args from the first instance), or stripping known deep-link arguments before relaunching.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/desktop/src/app/DesktopDeepLinks.ts around line 106:
On startup, `configure` re-parses `process.argv` and treats any preserved `t3://...` argument as a fresh deep link. The app's own relaunch path restarts packaged builds with `args: process.argv.slice(1)`, so if the app was originally launched from a `t3://` deep link, subsequent relaunches (for example WSL/server-exposure changes) carry the stale URL forward and reopen that old thread unexpectedly. Consider checking only argv that was not carried over from a prior launch (for example the OS-provided launch args from the first instance), or stripping known deep-link arguments before relaunching.
| ipcRenderer.removeListener(IpcChannels.MENU_ACTION_CHANNEL, wrappedListener); | ||
| }; | ||
| }, | ||
| onOpenThread: (listener) => { |
There was a problem hiding this comment.
🟠 High src/preload.ts:119
The onOpenThread listener is registered during useEffect in AppSidebarLayout, which runs after the renderer has loaded. The main process emits the thread-open event on did-finish-load, so a t3://thread/... deep link opened during cold start fires before this listener exists and is permanently dropped. Consider having the main process buffer early thread-open events (or replay the last one) so a late-subscribing renderer still receives them.
Also found in 1 other location(s)
apps/web/src/components/AppSidebarLayout.tsx:101
AppSidebarLayoutregisterswindow.desktopBridge.onOpenThreadinsideuseEffect, so the listener is attached only after the initial React commit. On a cold start from at3://thread/...link, the main process flushes the queued deep link ondid-finish-loadand the preload bridge forwards it immediately, but there is no replay/buffer inpreload.ts. That means the firstOPEN_THREAD_CHANNELevent can fire before this effect runs, and the deep link is silently dropped instead of navigating to the thread.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/desktop/src/preload.ts around line 119:
The `onOpenThread` listener is registered during `useEffect` in `AppSidebarLayout`, which runs after the renderer has loaded. The main process emits the thread-open event on `did-finish-load`, so a `t3://thread/...` deep link opened during cold start fires before this listener exists and is permanently dropped. Consider having the main process buffer early thread-open events (or replay the last one) so a late-subscribing renderer still receives them.
Also found in 1 other location(s):
- apps/web/src/components/AppSidebarLayout.tsx:101 -- `AppSidebarLayout` registers `window.desktopBridge.onOpenThread` inside `useEffect`, so the listener is attached only after the initial React commit. On a cold start from a `t3://thread/...` link, the main process flushes the queued deep link on `did-finish-load` and the preload bridge forwards it immediately, but there is no replay/buffer in `preload.ts`. That means the first `OPEN_THREAD_CHANNEL` event can fire before this effect runs, and the deep link is silently dropped instead of navigating to the thread.
2 participants
What Changed
Adds a
t3://thread/<threadId>OS-level protocol handler to the Electron desktop app.apps/desktop/src/main.ts: registersopen-url(macOS) andsecond-instance(Win/Linux) listeners beforewhenReady;handleDeepLink()parses the URL and sendsdesktop:open-threadto the renderer viawebContents.send;pendingDeepLinkThreadIdqueues deep links that arrive before the window finishes loading;setAsDefaultProtocolClient('t3')called inwhenReadyapps/desktop/src/preload.ts: exposesonOpenThreadover the context bridge (same pattern asonMenuAction)packages/contracts/src/ipc.ts: addsonOpenThreadto theDesktopBridgeinterfaceapps/web/src/components/AppSidebarLayout.tsx: subscribes toonOpenThreadand navigates to/$environmentId/$threadIdusing TanStack Routerscripts/build-desktop-artifact.ts: addsprotocols: [{name:"T3 Code", schemes:["t3"]}]to the mac electron-builder config soCFBundleURLTypesis written into the packagedInfo.plistlocalApi.test.ts,SettingsPanels.browser.tsx): stubonOpenThread: () => () => {}addedWhy
Clicking a thread link (e.g. from Slack or the terminal) currently opens a browser tab that hits the pairing screen. With this change,
t3://thread/<id>links route directly to the correct thread inside the already-running desktop app — or launch it if it's closed.Verified end-to-end in dev:
open "t3://thread/<id>"in the terminal fired theopen-urlevent,handleDeepLinkparsed it correctly, andwebContents.send("desktop:open-thread", threadId)was confirmed in the Electron main-process logs.UI Changes
N/A — no visual changes. Navigation behavior only.
Checklist
Note
Medium Risk
Changes OS protocol registration, startup ordering, and bundle IDs (install identity / multi-channel installs); deep-link parsing is narrow but affects how external URLs drive navigation.
Overview
Adds
t3://thread/<threadId>as the desktop thread deep link. A newDesktopDeepLinksservice registersopen-url/second-instanceearly, registers thet3protocol after ready (macOS pinssetAsDefaultProtocolClienttoprocess.execPath), parses URLs viathreadDeepLink, and opens threads throughDesktopWindow.openThread, which IPCsdesktop:open-threadwhen the main window is ready (with pending-thread queuing).The web shell listens via
onOpenThreadondesktopBridgeand navigates to/$environmentId/$threadId, deferring until an environment id is known.Alpha and Nightly packaged builds now use
com.t3tools.t3code.alpha/.nightlybundle IDs (dev unchanged); launcher and electron-builder registert3plus legacyt3codeschemes. Local artifact builds setpublish: nullso GH_TOKEN does not auto-enable GitHub publish.Reviewed by Cursor Bugbot for commit 75a2103. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Register
t3://OS protocol handler for thread deep links in the desktop appt3://thread/<threadId>deep link handling end-to-end: the OS registers the app as the defaultt3protocol client, parses incoming URLs, and routes them toDesktopWindow.openThread.DesktopDeepLinkshandles bothopen-url(macOS) andsecond-instance(Windows) Electron events, plus initial argv scanning for cold-start deep links.DesktopWindowbuffers thread open requests until the renderer is loaded, creating the window if needed, then sends via the newdesktop:open-threadIPC channel.AppSidebarLayoutsubscribes toonOpenThreadand navigates to the thread route, deferring until the active environment is resolved.com.t3tools.t3code.alphafor production,.nightlyfor nightly builds), and thet3scheme is added to macOSprotocolsin the electron-builder config.📊 Macroscope summarized 75a2103. 13 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.