Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { BrowserWindow, shell } from "electron";
import { registerRoute } from "lib/window-loader";
import type { WindowProps } from "shared/types";

export function createWindow({ id, ...settings }: WindowProps) {
export function createWindow({ id, path, query, ...settings }: WindowProps) {
const window = new BrowserWindow(settings);

// Open external URLs in the system browser instead of Electron
Expand All @@ -19,6 +19,8 @@ export function createWindow({ id, ...settings }: WindowProps) {
id,
browserWindow: window,
htmlFile: join(__dirname, "../renderer/index.html"),
path,
query,
});

window.on("closed", window.destroy);
Expand Down
6 changes: 5 additions & 1 deletion apps/desktop/src/lib/trpc/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,15 @@ import superjson from "superjson";
import type { AppRouter } from "./routers";
import { NotGitRepoError } from "./routers/workspaces/utils/git";

export interface TrpcContext {
windowId: number | null;
}

/**
* Core tRPC initialization
* This provides the base router and procedure builders used by all routers
*/
const t = initTRPC.create({
const t = initTRPC.context<TrpcContext>().create({
transformer: superjson,
isServer: true,
});
Expand Down
12 changes: 8 additions & 4 deletions apps/desktop/src/lib/trpc/routers/ui-state/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { observable } from "@trpc/server/observable";
import { appState } from "main/lib/app-state";
import type { TabsState, ThemeState } from "main/lib/app-state/schemas";
import {
getTabsStateForWindow,
setTabsStateForWindow,
} from "main/lib/app-state/tabs-state";
import { hotkeysEmitter } from "main/lib/hotkeys-events";
import {
buildOverridesFromBindings,
Expand Down Expand Up @@ -235,14 +239,14 @@ export const createUiStateRouter = () => {
return router({
// Tabs state procedures
tabs: router({
get: publicProcedure.query((): TabsState => {
return appState.data.tabsState;
get: publicProcedure.query(({ ctx }): TabsState => {
return getTabsStateForWindow(ctx.windowId);
}),

set: publicProcedure
.input(tabsStateSchema)
.mutation(async ({ input }) => {
appState.data.tabsState = input;
.mutation(async ({ ctx, input }) => {
setTabsStateForWindow(ctx.windowId, input);
await appState.write();
return { success: true };
}),
Expand Down
14 changes: 14 additions & 0 deletions apps/desktop/src/lib/trpc/routers/window.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { homedir } from "node:os";
import path from "node:path";
import type { BrowserWindow } from "electron";
import { dialog } from "electron";
import { openWorkspaceWindow } from "main/lib/window-manager";
import { z } from "zod";
import { publicProcedure, router } from "..";

Expand Down Expand Up @@ -47,6 +48,19 @@ export const createWindowRouter = (getWindow: () => BrowserWindow | null) => {
return homedir();
}),

openWorkspaceWindow: publicProcedure
.input(
z.object({
workspaceId: z.string().min(1),
tabId: z.string().optional(),
paneId: z.string().optional(),
}),
)
.mutation(({ input }) => {
openWorkspaceWindow(input);
return { success: true };
}),

selectDirectory: publicProcedure
.input(
z
Expand Down
11 changes: 9 additions & 2 deletions apps/desktop/src/lib/window-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,27 @@ export function registerRoute(props: {
id: WindowId;
browserWindow: BrowserWindow;
htmlFile: string;
path?: string;
query?: Record<string, string>;
}): void {
const isDev = env.NODE_ENV === "development";
const path = props.path ?? "/";
const normalizedPath = path.startsWith("/") ? path : `/${path}`;
const search = props.query
? new URLSearchParams(props.query).toString()
: "";
const hash = search ? `${normalizedPath}?${search}` : normalizedPath;

if (isDev) {
// Development: load from Vite dev server with hash routing
const url = `http://localhost:${env.DESKTOP_VITE_PORT}/#/`;
const url = `http://localhost:${env.DESKTOP_VITE_PORT}/#${hash}`;
console.log("[window-loader] Loading development URL:", url);
props.browserWindow.loadURL(url);
} else {
// Production: load from file with hash routing
// TanStack Router uses hash-based routing, so we always start at #/
console.log("[window-loader] Loading file:", props.htmlFile);
props.browserWindow.loadFile(props.htmlFile, { hash: "/" });
props.browserWindow.loadFile(props.htmlFile, { hash });
}

// Log successful loads
Expand Down
4 changes: 4 additions & 0 deletions apps/desktop/src/main/lib/app-state/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ function ensureValidShape(data: Partial<AppState>): AppState {
...defaultAppState.tabsState,
...(data.tabsState ?? {}),
},
tabsStateByWindow: {
...defaultAppState.tabsStateByWindow,
...(data.tabsStateByWindow ?? {}),
},
themeState: {
...defaultAppState.themeState,
...(data.themeState ?? {}),
Expand Down
8 changes: 8 additions & 0 deletions apps/desktop/src/main/lib/app-state/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,20 @@ import type { Theme } from "shared/themes";
// Re-export for convenience
export type { BaseTabsState as TabsState, Pane } from "shared/tabs-types";

export interface WindowTabsState {
activeTabIds: BaseTabsState["activeTabIds"];
focusedPaneIds: BaseTabsState["focusedPaneIds"];
tabHistoryStacks: BaseTabsState["tabHistoryStacks"];
}

export interface ThemeState {
activeThemeId: string;
customThemes: Theme[];
}

export interface AppState {
tabsState: BaseTabsState;
tabsStateByWindow: Record<string, WindowTabsState>;
themeState: ThemeState;
hotkeysState: HotkeysState;
}
Expand All @@ -27,6 +34,7 @@ export const defaultAppState: AppState = {
focusedPaneIds: {},
tabHistoryStacks: {},
},
tabsStateByWindow: {},
themeState: {
activeThemeId: "dark",
customThemes: [],
Expand Down
99 changes: 99 additions & 0 deletions apps/desktop/src/main/lib/app-state/tabs-state.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import { appState } from ".";
import {
defaultAppState,
type TabsState,
type WindowTabsState,
} from "./schemas";

function getWindowKey(windowId: number | null | undefined): string | null {
return windowId === null || windowId === undefined ? null : String(windowId);
}

function toWindowTabsState(
state: Partial<TabsState> | undefined,
): WindowTabsState {
return {
activeTabIds: state?.activeTabIds ?? {},
focusedPaneIds: state?.focusedPaneIds ?? {},
tabHistoryStacks: state?.tabHistoryStacks ?? {},
};
Comment on lines +12 to +19
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

toWindowTabsState currently aliases maps, causing merge-time mutation side effects.

Lines 16-18 return original map references. Then Lines 46-48 mutate those objects via Object.assign, which can unintentionally mutate shared persisted state during read/merge.

💡 Proposed fix
 function toWindowTabsState(
 	state: Partial<TabsState> | undefined,
 ): WindowTabsState {
 	return {
-		activeTabIds: state?.activeTabIds ?? {},
-		focusedPaneIds: state?.focusedPaneIds ?? {},
-		tabHistoryStacks: state?.tabHistoryStacks ?? {},
+		activeTabIds: { ...(state?.activeTabIds ?? {}) },
+		focusedPaneIds: { ...(state?.focusedPaneIds ?? {}) },
+		tabHistoryStacks: { ...(state?.tabHistoryStacks ?? {}) },
 	};
 }

Also applies to: 40-49

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/main/lib/app-state/tabs-state.ts` around lines 12 - 19,
toWindowTabsState is returning references to the original maps which leads to
merge-time mutation; change it to return new shallow copies instead of aliasing
(e.g., replace activeTabIds: state?.activeTabIds ?? {} with activeTabIds: {
...state?.activeTabIds } ?? {} and do the same for focusedPaneIds and
tabHistoryStacks) so later uses of Object.assign or mutations (see the places
that call toWindowTabsState and the Object.assign merge code) will not mutate
persisted/shared state; also ensure any Object.assign calls create a new target
object (Object.assign({}, existing, updates)) rather than mutating an input
object.

}

export function getTabsStateForWindow(
windowId: number | null | undefined,
): TabsState {
const sharedState = appState.data.tabsState ?? defaultAppState.tabsState;
const key = getWindowKey(windowId);
const windowState =
(key ? appState.data.tabsStateByWindow[key] : undefined) ??
toWindowTabsState(sharedState);
Comment on lines +22 to +29
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Guard tabsStateByWindow before keyed access to avoid crash.

On Line 28, appState.data.tabsStateByWindow[key] can throw if older/malformed persisted state is loaded without this property.

💡 Proposed fix
 export function getTabsStateForWindow(
 	windowId: number | null | undefined,
 ): TabsState {
 	const sharedState = appState.data.tabsState ?? defaultAppState.tabsState;
 	const key = getWindowKey(windowId);
+	const byWindow = appState.data.tabsStateByWindow ?? {};
 	const windowState =
-		(key ? appState.data.tabsStateByWindow[key] : undefined) ??
+		(key ? byWindow[key] : undefined) ??
 		toWindowTabsState(sharedState);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export function getTabsStateForWindow(
windowId: number | null | undefined,
): TabsState {
const sharedState = appState.data.tabsState ?? defaultAppState.tabsState;
const key = getWindowKey(windowId);
const windowState =
(key ? appState.data.tabsStateByWindow[key] : undefined) ??
toWindowTabsState(sharedState);
export function getTabsStateForWindow(
windowId: number | null | undefined,
): TabsState {
const sharedState = appState.data.tabsState ?? defaultAppState.tabsState;
const key = getWindowKey(windowId);
const byWindow = appState.data.tabsStateByWindow ?? {};
const windowState =
(key ? byWindow[key] : undefined) ??
toWindowTabsState(sharedState);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/main/lib/app-state/tabs-state.ts` around lines 22 - 29, The
function getTabsStateForWindow reads appState.data.tabsStateByWindow[key]
without verifying tabsStateByWindow exists; guard access to avoid crashes when
older/malformed persisted state lacks that property by checking
appState.data.tabsStateByWindow (or using optional chaining) before indexing
with key, and fall back to toWindowTabsState(sharedState) when tabsStateByWindow
is missing or key lookup yields undefined; update the windowState assignment in
getTabsStateForWindow accordingly so it references
appState.data.tabsStateByWindow only after confirming that object exists.


return {
tabs: sharedState.tabs,
panes: sharedState.panes,
activeTabIds: windowState.activeTabIds,
focusedPaneIds: windowState.focusedPaneIds,
tabHistoryStacks: windowState.tabHistoryStacks,
};
}

function getMergedWindowTabsState(): WindowTabsState {
const merged = toWindowTabsState(appState.data.tabsState);
const byWindow = appState.data.tabsStateByWindow;

for (const state of Object.values(byWindow ?? {})) {
const windowState = toWindowTabsState(state);
Object.assign(merged.activeTabIds, windowState.activeTabIds);
Object.assign(merged.focusedPaneIds, windowState.focusedPaneIds);
Object.assign(merged.tabHistoryStacks, windowState.tabHistoryStacks);
}

return merged;
}

export function setTabsStateForWindow(
windowId: number | null | undefined,
tabsState: TabsState,
): void {
const windowState: WindowTabsState = {
activeTabIds: tabsState.activeTabIds,
focusedPaneIds: tabsState.focusedPaneIds,
tabHistoryStacks: tabsState.tabHistoryStacks,
};

const key = getWindowKey(windowId);
if (key) {
appState.data.tabsStateByWindow = {
...appState.data.tabsStateByWindow,
[key]: windowState,
};
}

// Shared across windows: tab/pane topology.
// Legacy fallback keeps the latest caller's view state.
appState.data.tabsState = {
tabs: tabsState.tabs,
panes: tabsState.panes,
activeTabIds: windowState.activeTabIds,
focusedPaneIds: windowState.focusedPaneIds,
tabHistoryStacks: windowState.tabHistoryStacks,
};
}

export function getMergedTabsState(): TabsState {
const sharedState = appState.data.tabsState ?? defaultAppState.tabsState;
const mergedWindowState = getMergedWindowTabsState();

return {
tabs: sharedState.tabs,
panes: sharedState.panes,
activeTabIds: mergedWindowState.activeTabIds,
focusedPaneIds: mergedWindowState.focusedPaneIds,
tabHistoryStacks: mergedWindowState.tabHistoryStacks,
};
}

export function resetTabsState(): void {
appState.data.tabsState = defaultAppState.tabsState;
appState.data.tabsStateByWindow = {};
Comment on lines +96 to +98
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Avoid assigning defaultAppState.tabsState by reference during reset.

Line 97 reuses the same object instance from defaults; later in-place mutations can corrupt the canonical default object.

💡 Proposed fix
 export function resetTabsState(): void {
-	appState.data.tabsState = defaultAppState.tabsState;
+	appState.data.tabsState = {
+		tabs: [...defaultAppState.tabsState.tabs],
+		panes: { ...defaultAppState.tabsState.panes },
+		activeTabIds: { ...defaultAppState.tabsState.activeTabIds },
+		focusedPaneIds: { ...defaultAppState.tabsState.focusedPaneIds },
+		tabHistoryStacks: { ...defaultAppState.tabsState.tabHistoryStacks },
+	};
 	appState.data.tabsStateByWindow = {};
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/main/lib/app-state/tabs-state.ts` around lines 96 - 98, The
resetTabsState function currently assigns appState.data.tabsState =
defaultAppState.tabsState by reference which can let future in-place mutations
corrupt the canonical default; replace that assignment with a deep copy (e.g.,
structuredClone or an equivalent deep-clone utility) so appState.data.tabsState
becomes an independent object, and keep the existing clearing of
appState.data.tabsStateByWindow; update references in resetTabsState to use the
cloned value rather than the original default object.

}
26 changes: 18 additions & 8 deletions apps/desktop/src/main/lib/menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { env } from "main/env.main";
import { appState } from "main/lib/app-state";
import { hotkeysEmitter } from "main/lib/hotkeys-events";
import { resetTerminalStateDev } from "main/lib/terminal/dev-reset";
import { openLastActiveWorkspaceWindow } from "main/lib/window-manager";
import {
getCurrentPlatform,
getEffectiveHotkey,
Expand Down Expand Up @@ -38,6 +39,7 @@ export function registerMenuHotkeyUpdates() {

export function createApplicationMenu() {
const closeAccelerator = getMenuAccelerator("CLOSE_WINDOW");
const newWindowAccelerator = getMenuAccelerator("NEW_WINDOW");
const showHotkeysAccelerator = getMenuAccelerator("SHOW_HOTKEYS");
const openSettingsAccelerator = getMenuAccelerator("OPEN_SETTINGS");

Expand Down Expand Up @@ -68,14 +70,22 @@ export function createApplicationMenu() {
{ role: "togglefullscreen" },
],
},
{
label: "Window",
submenu: [
{ role: "minimize" },
{ role: "zoom" },
{ type: "separator" },
{ role: "close", accelerator: closeAccelerator },
],
{
label: "Window",
submenu: [
{
label: "New Window",
accelerator: newWindowAccelerator,
click: () => {
openLastActiveWorkspaceWindow();
},
},
{ type: "separator" },
{ role: "minimize" },
{ role: "zoom" },
{ type: "separator" },
{ role: "close", accelerator: closeAccelerator },
],
},
{
label: "Help",
Expand Down
4 changes: 2 additions & 2 deletions apps/desktop/src/main/lib/notifications/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { handleAuthCallback } from "lib/trpc/routers/auth/utils/auth-functions";
import { NOTIFICATION_EVENTS } from "shared/constants";
import { env } from "shared/env.shared";
import type { AgentLifecycleEvent } from "shared/notification-types";
import { appState } from "../app-state";
import { getMergedTabsState } from "../app-state/tabs-state";
import { HOOK_PROTOCOL_VERSION } from "../terminal/env";
import { mapEventType } from "./map-event-type";

Expand Down Expand Up @@ -59,7 +59,7 @@ function resolvePaneId(
sessionId: string | undefined,
): string | undefined {
try {
const tabsState = appState.data.tabsState;
const tabsState = getMergedTabsState();
if (!tabsState) return undefined;

// If paneId provided, validate it exists before returning
Expand Down
4 changes: 2 additions & 2 deletions apps/desktop/src/main/lib/terminal/daemon/daemon-manager.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { EventEmitter } from "node:events";
import { workspaces } from "@superset/local-db";
import { track } from "main/lib/analytics";
import { appState } from "main/lib/app-state";
import { getMergedTabsState } from "main/lib/app-state/tabs-state";
import { localDb } from "main/lib/local-db";
import { HistoryReader, truncateUtf8ToLastBytes } from "../../terminal-history";
import {
Expand Down Expand Up @@ -552,7 +552,7 @@ export class DaemonTerminalManager extends EventEmitter {

private getCreateOrAttachPriority(params: CreateSessionParams): number {
try {
const tabsState = appState.data?.tabsState;
const tabsState = getMergedTabsState();
const activeTabId = tabsState?.activeTabIds?.[params.workspaceId];
const focusedPaneId =
activeTabId && tabsState?.focusedPaneIds?.[activeTabId];
Expand Down
4 changes: 2 additions & 2 deletions apps/desktop/src/main/lib/terminal/dev-reset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { rm } from "node:fs/promises";
import { join } from "node:path";
import { SUPERSET_HOME_DIR } from "main/lib/app-environment";
import { appState } from "main/lib/app-state";
import { defaultAppState } from "main/lib/app-state/schemas";
import { resetTabsState } from "main/lib/app-state/tabs-state";
import {
disposeTerminalHostClient,
getTerminalHostClient,
Expand Down Expand Up @@ -45,7 +45,7 @@ export async function resetTerminalStateDev(): Promise<void> {
}

// Clear tabs/panes so we don't immediately try to restore a large terminal set.
appState.data.tabsState = defaultAppState.tabsState;
resetTabsState();
try {
await appState.write();
} catch (error) {
Expand Down
Loading
Loading