Skip to content

Revert "fix(desktop): queue early macOS deep links to prevent auth redirect loss" #1440

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Kitenite wants to merge 1 commit into from
Closed

Revert "fix(desktop): queue early macOS deep links to prevent auth redirect loss" #1440

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 57 additions & 27 deletions apps/desktop/src/main/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,16 @@ import { reconcileDaemonSessions } from "./lib/terminal";
import { disposeTray, initTray } from "./lib/tray";
import { MainWindow } from "./windows/main";

// Initialize local SQLite database (runs migrations + legacy data migration on import)
console.log("[main] Local database ready:", !!localDb);

const workspaceName = getWorkspaceName();
if (workspaceName) {
app.setName(`Superset (${workspaceName})`);
}

// Dev mode: register with execPath + app script so macOS launches Electron with our entry point
// Register protocol handler for deep linking
// In development, we need to provide the execPath and args
if (process.defaultApp) {
if (process.argv.length >= 2) {
app.setAsDefaultProtocolClient(PROTOCOL_SCHEME, process.execPath, [
Expand All @@ -40,6 +42,7 @@ if (process.defaultApp) {
async function processDeepLink(url: string): Promise<void> {
console.log("[main] Processing deep link:", url);

// Try auth deep link first (special handling)
const authParams = parseAuthDeepLink(url);
if (authParams) {
const result = await handleAuthCallback(authParams);
Expand All @@ -51,21 +54,32 @@ async function processDeepLink(url: string): Promise<void> {
return;
}

// Non-auth deep links: extract path and navigate in renderer
// For all other deep links, extract path and navigate in renderer
// e.g. superset://tasks/my-slug -> /tasks/my-slug
// e.g. superset://settings/integrations -> /settings/integrations
const path = `/${url.split("://")[1]}`;

focusMainWindow();

// Navigate in renderer via loading the route directly
const windows = BrowserWindow.getAllWindows();
if (windows.length > 0) {
windows[0].webContents.send("deep-link-navigate", path);
const mainWindow = windows[0];
// Send navigation request to renderer
mainWindow.webContents.send("deep-link-navigate", path);
}
}

/**
* Find a deep link URL in argv
*/
function findDeepLinkInArgv(argv: string[]): string | undefined {
return argv.find((arg) => arg.startsWith(`${PROTOCOL_SCHEME}://`));
}

/**
* Focus the main window (show and bring to front)
*/
function focusMainWindow(): void {
const windows = BrowserWindow.getAllWindows();
if (windows.length > 0) {
Expand All @@ -78,23 +92,18 @@ function focusMainWindow(): void {
}
}

// macOS open-url can fire before the window exists (cold-start via protocol link).
// Queue the URL and process it after initialization.
let pendingDeepLinkUrl: string | null = null;
let appReady = false;

// Handle deep links when app is already running (macOS)
app.on("open-url", async (event, url) => {
event.preventDefault();
if (appReady) {
await processDeepLink(url);
} else {
pendingDeepLinkUrl = url;
}
await processDeepLink(url);
});
Comment on lines +95 to 99
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

macOS cold-start deep links will be silently lost.

On macOS, open-url fires before app.whenReady(). At that point:

  • Auth deep links: handleAuthCallback may not have the required state (e.g., initAppState at line 266 hasn't run yet).
  • Non-auth deep links: BrowserWindow.getAllWindows() returns [], so the navigation at line 66–70 is silently skipped.

Windows/Linux cold-start deep links are handled correctly via findDeepLinkInArgv at lines 285–289 (post-init), but macOS has no equivalent deferred path after this revert.

If the revert is intentional and temporary, consider adding a // TODO here documenting the known gap and linking to the follow-up issue.

🤖 Prompt for AI Agents 
In `@apps/desktop/src/main/index.ts` around lines 95 - 99, The macOS open-url
handler (app.on("open-url", ...) calling processDeepLink) can fire before
initialization (initAppState) and before windows exist, causing cold-start deep
links to be lost; modify the handler to detect when the app is not ready or
initAppState hasn't run and stash the incoming URL (e.g., in a module-level
pendingDeepLink variable or queue) and then process it after startup (reuse
existing processDeepLink call in the post-init path that runs
findDeepLinkInArgv), ensuring handleAuthCallback can access initialized state
and BrowserWindow.getAllWindows() will find windows; if this is intentionally
deferred, add a TODO comment here referencing the known gap and link to the
follow-up issue.


let isQuitting = false;
let skipConfirmation = false;

/**
* Check if the user has enabled the confirm-on-quit setting
*/
function getConfirmOnQuitSetting(): boolean {
try {
const row = localDb.select().from(settings).get();
Expand All @@ -104,10 +113,16 @@ function getConfirmOnQuitSetting(): boolean {
}
}

/**
* Skip the confirmation dialog for the next quit (e.g., auto-updater)
*/
export function setSkipQuitConfirmation(): void {
skipConfirmation = true;
}

/**
* Skip the confirmation dialog and quit immediately
*/
export function quitWithoutConfirmation(): void {
skipConfirmation = true;
app.exit(0);
Expand All @@ -133,12 +148,17 @@ app.on("before-quit", async (event) => {
message: "Are you sure you want to quit?",
});

if (response === 1) return;
if (response === 1) {
// User cancelled
return;
}
} catch (error) {
console.error("[main] Quit confirmation dialog failed:", error);
}
}

// Quit confirmed or no confirmation needed - exit immediately
// Let OS clean up child processes, tray, etc.
isQuitting = true;
disposeTray();
app.exit(0);
Expand All @@ -154,20 +174,26 @@ process.on("unhandledRejection", (reason) => {
console.error("[main] Unhandled rejection:", reason);
});

// Handle termination signals (e.g., when dev server stops via Ctrl+C)
// Without these handlers, Electron may not quit when electron-vite sends SIGTERM
if (process.env.NODE_ENV === "development") {
const handleTerminationSignal = (signal: string) => {
console.log(`[main] Received ${signal}, quitting...`);
// Use app.exit() to bypass before-quit async cleanup which can hang
app.exit(0);
};

process.on("SIGTERM", () => handleTerminationSignal("SIGTERM"));
process.on("SIGINT", () => handleTerminationSignal("SIGINT"));

// Fallback: electron-vite may exit without signaling the child Electron process
// Monitor parent process (electron-vite CLI) and quit if it exits.
// This is a fallback for when signals don't propagate correctly.
// When electron-vite receives Ctrl+C, it may exit without properly
// signaling the child Electron process to quit.
const parentPid = process.ppid;
const isParentAlive = (): boolean => {
const checkParentAlive = (): boolean => {
try {
// Signal 0 doesn't actually send a signal, just checks if process exists
process.kill(parentPid, 0);
return true;
} catch {
Expand All @@ -176,15 +202,18 @@ if (process.env.NODE_ENV === "development") {
};

const parentCheckInterval = setInterval(() => {
if (!isParentAlive()) {
if (!checkParentAlive()) {
console.log("[main] Parent process exited, quitting...");
clearInterval(parentCheckInterval);
// Use app.exit() instead of app.quit() to bypass the before-quit
// handler's async cleanup which can hang in dev mode
app.exit(0);
}
}, 1000);
parentCheckInterval.unref();
}

// Register superset-icon:// protocol for serving project icons from disk
protocol.registerSchemesAsPrivileged([
{
scheme: "superset-icon",
Expand All @@ -202,7 +231,6 @@ const gotTheLock = app.requestSingleInstanceLock();
if (!gotTheLock) {
app.exit(0);
} else {
// Windows/Linux: protocol URL arrives as argv on the second instance
app.on("second-instance", async (_event, argv) => {
focusMainWindow();
const url = findDeepLinkInArgv(argv);
Expand All @@ -214,9 +242,11 @@ if (!gotTheLock) {
(async () => {
await app.whenReady();

// Must register on both default session and the app's custom partition
// Register protocol handler for superset-icon:// URLs
// Must register on BOTH default session and the app's custom partition
const iconProtocolHandler = (request: Request) => {
const url = new URL(request.url);
// superset-icon://projects/{projectId} → file on disk
const projectId = url.pathname.replace(/^\//, "");
const iconPath = getProjectIconPath(projectId);
if (!iconPath) {
Expand All @@ -230,32 +260,32 @@ if (!gotTheLock) {
.protocol.handle("superset-icon", iconProtocolHandler);

ensureProjectIconsDir();

initSentry();

await initAppState();

// Must happen before renderer restore runs
// Clean up stale daemon sessions from previous app runs
// Must happen BEFORE renderer restore runs
await reconcileDaemonSessions();

try {
setupAgentHooks();
} catch (error) {
console.error("[main] Failed to set up agent hooks:", error);
// App can continue without agent hooks, but log the failure
}

await makeAppSetup(() => MainWindow());
setupAutoUpdater();

// Initialize system tray (macOS menu bar icon for daemon management)
initTray();

// Process any deep links from cold start
// Handle cold-start deep links (Windows/Linux - app launched via deep link)
const coldStartUrl = findDeepLinkInArgv(process.argv);
if (coldStartUrl) {
await processDeepLink(coldStartUrl);
}
if (pendingDeepLinkUrl) {
await processDeepLink(pendingDeepLinkUrl);
pendingDeepLinkUrl = null;
}

appReady = true;
})();
}