Skip to content
Open
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
2 changes: 2 additions & 0 deletions apps/desktop/src/app/DesktopEnvironment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export class DesktopEnvironment extends Context.Service<
readonly clientSettingsPath: string;
readonly savedEnvironmentRegistryPath: string;
readonly serverSettingsPath: string;
readonly windowStatePath: string;
readonly logDir: string;
readonly browserArtifactsDir: string;
readonly rootDir: string;
Expand Down Expand Up @@ -178,6 +179,7 @@ const make = Effect.fn("desktop.environment.make")(function* (
clientSettingsPath: path.join(stateDir, "client-settings.json"),
savedEnvironmentRegistryPath: path.join(stateDir, "saved-environments.json"),
serverSettingsPath: path.join(stateDir, "settings.json"),
windowStatePath: path.join(stateDir, "window-state.json"),
logDir: path.join(stateDir, "logs"),
browserArtifactsDir: path.join(stateDir, "browser-artifacts"),
rootDir,
Expand Down
2 changes: 2 additions & 0 deletions apps/desktop/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import * as DesktopUpdates from "./updates/DesktopUpdates.ts";
import * as BrowserSession from "./preview/BrowserSession.ts";
import * as PreviewManager from "./preview/Manager.ts";
import * as DesktopWindow from "./window/DesktopWindow.ts";
import * as DesktopWindowState from "./window/DesktopWindowState.ts";
import * as DesktopWslBackend from "./wsl/DesktopWslBackend.ts";
import * as DesktopWslEnvironment from "./wsl/DesktopWslEnvironment.ts";

Expand Down Expand Up @@ -124,6 +125,7 @@ const desktopFoundationLayer = Layer.mergeAll(
DesktopConnectionCatalogStore.layer.pipe(Layer.provideMerge(DesktopSavedEnvironments.layer)),
DesktopAssets.layer,
DesktopObservability.layer,
DesktopWindowState.layer,
).pipe(Layer.provideMerge(desktopEnvironmentLayer));

const desktopSshLayer = desktopSshEnvironmentLayer.pipe(
Expand Down
15 changes: 15 additions & 0 deletions apps/desktop/src/window/DesktopWindow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ vi.mock("electron", async (importOriginal) => ({
setUserAgent: vi.fn(),
})),
},
screen: {
getAllDisplays: vi.fn(() => []),
},
}));

import * as DesktopAssets from "../app/DesktopAssets.ts";
Expand All @@ -31,6 +34,7 @@ import * as ElectronWindow from "../electron/ElectronWindow.ts";
import { MENU_ACTION_CHANNEL } from "../ipc/channels.ts";
import * as DesktopServerExposure from "../backend/DesktopServerExposure.ts";
import * as DesktopWindow from "./DesktopWindow.ts";
import * as DesktopWindowState from "./DesktopWindowState.ts";
import * as PreviewManager from "../preview/Manager.ts";

const environmentInput = {
Expand Down Expand Up @@ -65,10 +69,14 @@ function makeFakeBrowserWindow() {
const window = {
close: vi.fn(),
focus: vi.fn(),
getNormalBounds: vi.fn(() => ({ x: 0, y: 0, width: 1100, height: 780 })),
isDestroyed: vi.fn(() => false),
isFullScreen: vi.fn(() => false),
isMaximized: vi.fn(() => false),
isMinimized: vi.fn(() => false),
isVisible: vi.fn(() => true),
loadURL: vi.fn(() => Promise.resolve()),
maximize: vi.fn(),
on: vi.fn(),
once: vi.fn(),
restore: vi.fn(),
Expand Down Expand Up @@ -127,6 +135,11 @@ const electronThemeLayer = Layer.succeed(ElectronTheme.ElectronTheme, {
onUpdated: () => Effect.void,
} satisfies ElectronTheme.ElectronTheme["Service"]);

const desktopWindowStateLayer = Layer.succeed(DesktopWindowState.DesktopWindowState, {
load: Effect.succeed(Option.none<DesktopWindowState.WindowState>()),
save: () => Effect.void,
} satisfies DesktopWindowState.DesktopWindowState["Service"]);

const desktopEnvironmentLayer = DesktopEnvironment.layer(environmentInput).pipe(
Layer.provide(
Layer.mergeAll(
Expand Down Expand Up @@ -171,6 +184,7 @@ function makeTestLayer(input: {
desktopAssetsLayer,
desktopEnvironmentLayer,
desktopServerExposureLayer,
desktopWindowStateLayer,
DesktopState.layer,
electronMenuLayer,
Layer.succeed(ElectronShell.ElectronShell, {
Expand Down Expand Up @@ -268,6 +282,7 @@ const makeSplashScenario = (createOutcomes: readonly (Electron.BrowserWindow | n
desktopAssetsLayer,
desktopEnvironmentLayer,
desktopServerExposureLayer,
desktopWindowStateLayer,
electronMenuLayer,
Layer.succeed(ElectronShell.ElectronShell, {
openExternal: () => Effect.succeed(true),
Expand Down
77 changes: 75 additions & 2 deletions apps/desktop/src/window/DesktopWindow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import * as Option from "effect/Option";
import * as Ref from "effect/Ref";

import type * as Electron from "electron";
import { screen } from "electron";

import * as DesktopAssets from "../app/DesktopAssets.ts";
import * as DesktopEnvironment from "../app/DesktopEnvironment.ts";
Expand All @@ -17,7 +18,12 @@ import * as ElectronTheme from "../electron/ElectronTheme.ts";
import * as ElectronWindow from "../electron/ElectronWindow.ts";
import { MENU_ACTION_CHANNEL } from "../ipc/channels.ts";
import * as PreviewManager from "../preview/Manager.ts";
import * as DesktopWindowState from "./DesktopWindowState.ts";

const DEFAULT_WINDOW_SIZE = { width: 1100, height: 780 } as const;
// Resize/move fire continuously while dragging; coalesce persistence so we write
// once the gesture settles instead of on every pixel.
const WINDOW_STATE_SAVE_DEBOUNCE_MS = 400;
const TITLEBAR_HEIGHT = 40;
const TITLEBAR_COLOR = "#01000000"; // #00000000 does not work correctly on Linux
const TITLEBAR_LIGHT_SYMBOL_COLOR = "#1f2937";
Expand Down Expand Up @@ -45,6 +51,7 @@ type DesktopWindowRuntimeServices =
| ElectronShell.ElectronShell
| ElectronTheme.ElectronTheme
| ElectronWindow.ElectronWindow
| DesktopWindowState.DesktopWindowState
| PreviewManager.PreviewManager;

export type DesktopWindowError =
Expand Down Expand Up @@ -201,6 +208,7 @@ export const make = Effect.gen(function* () {
const electronShell = yield* ElectronShell.ElectronShell;
const electronTheme = yield* ElectronTheme.ElectronTheme;
const electronWindow = yield* ElectronWindow.ElectronWindow;
const windowState = yield* DesktopWindowState.DesktopWindowState;
const previewManager = yield* PreviewManager.PreviewManager;
// Window-side latch for the primary backend's readiness. Set by
// handleBackendReady (driven by the pool's onReady callback), cleared
Expand Down Expand Up @@ -250,9 +258,14 @@ export const make = Effect.gen(function* () {
const iconPaths = yield* assets.iconPaths;
const iconOption = getIconOption(iconPaths, environment.platform);
const shouldUseDarkColors = yield* electronTheme.shouldUseDarkColors;
const savedWindowState = yield* windowState.load;
const initialWindowBounds = DesktopWindowState.resolveInitialWindowBounds(
savedWindowState,
screen.getAllDisplays().map((display) => display.workArea),
DEFAULT_WINDOW_SIZE,
);
const window = yield* electronWindow.create({
width: 1100,
height: 780,
...initialWindowBounds.bounds,
minWidth: 840,
minHeight: 620,
show: false,
Expand All @@ -275,6 +288,66 @@ export const make = Effect.gen(function* () {
window.setAutoHideCursor(false);
}

if (initialWindowBounds.maximize) {
window.maximize();
}

// Persist geometry so the next launch restores it. Saves are debounced via a
// restartable fiber (mirroring the development-load retry below) while the
// user drags/resizes, and flushed on close; a failed write is logged and
// otherwise ignored since it only costs the remembered size, nothing more.
let windowStateSaveFiber: Fiber.Fiber<void, never> | undefined;
const persistWindowState = () => {
if (window.isDestroyed() || window.isMinimized() || window.isFullScreen()) {
return;
Comment on lines +301 to +302

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.

🟡 Medium window/DesktopWindow.ts:301

persistWindowState bails out when window.isMinimized() is true, and the close handler calls it after canceling the pending debounce. When the user quits the app while the window is minimized, no state is written even though getNormalBounds() still returns valid restorable bounds in that state, so the next launch restores stale geometry. Consider removing the isMinimized() guard from persistWindowState so the final close always writes the current bounds.

Suggested change
if (window.isDestroyed() || window.isMinimized() || window.isFullScreen()) {
return;
if (window.isDestroyed() || window.isFullScreen()) {
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/desktop/src/window/DesktopWindow.ts around lines 301-302:

`persistWindowState` bails out when `window.isMinimized()` is true, and the `close` handler calls it after canceling the pending debounce. When the user quits the app while the window is minimized, no state is written even though `getNormalBounds()` still returns valid restorable bounds in that state, so the next launch restores stale geometry. Consider removing the `isMinimized()` guard from `persistWindowState` so the final close always writes the current bounds.

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.

Minimized window skips close persist

Medium Severity

When the main window closes while minimized, persistWindowState returns early, preventing the final window geometry from being saved. This can lead to stale window state persistence.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit cd5a7ba. Configure here.

}
const normalBounds = window.getNormalBounds();
runFork(
windowState
.save({
x: normalBounds.x,
y: normalBounds.y,
width: normalBounds.width,
height: normalBounds.height,
maximized: window.isMaximized(),
})
.pipe(
Effect.catch((error) =>
logWindowWarning("failed to persist window state", { path: error.path }),
),
),
);
};
const cancelScheduledWindowStateSave = () => {
if (windowStateSaveFiber === undefined) {
return;
}
const saveFiber = windowStateSaveFiber;
windowStateSaveFiber = undefined;
runFork(Fiber.interrupt(saveFiber));
};
const scheduleWindowStateSave = () => {
cancelScheduledWindowStateSave();
windowStateSaveFiber = runFork(
Effect.sleep(WINDOW_STATE_SAVE_DEBOUNCE_MS).pipe(
Effect.andThen(
Effect.sync(() => {
windowStateSaveFiber = undefined;
persistWindowState();
}),
),
),
);
};
window.on("resize", scheduleWindowStateSave);
window.on("move", scheduleWindowStateSave);
window.on("maximize", scheduleWindowStateSave);
window.on("unmaximize", scheduleWindowStateSave);
window.on("close", () => {
cancelScheduledWindowStateSave();
persistWindowState();
});

yield* previewManager.setMainWindow(window);
window.webContents.on("will-attach-webview", (event, webPreferences, params) => {
if (
Expand Down
49 changes: 49 additions & 0 deletions apps/desktop/src/window/DesktopWindowState.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { assert, describe, it } from "@effect/vitest";
import * as Option from "effect/Option";

import * as DesktopWindowState from "./DesktopWindowState.ts";

const DEFAULTS = { width: 1100, height: 780 } as const;
const DISPLAYS = [{ x: 0, y: 0, width: 1920, height: 1080 }] as const;

describe("resolveInitialWindowBounds", () => {
it("falls back to defaults when there is no saved state", () => {
assert.deepStrictEqual(
DesktopWindowState.resolveInitialWindowBounds(Option.none(), DISPLAYS, DEFAULTS),
{ bounds: { width: 1100, height: 780 }, maximize: false },
);
});

it("restores a saved position that is visible on a display", () => {
assert.deepStrictEqual(
DesktopWindowState.resolveInitialWindowBounds(
Option.some({ x: 100, y: 120, width: 800, height: 600, maximized: true }),
DISPLAYS,
DEFAULTS,
),
{ bounds: { x: 100, y: 120, width: 800, height: 600 }, maximize: true },
);
});

it("drops an off-screen position but keeps the size and maximized flag", () => {
assert.deepStrictEqual(
DesktopWindowState.resolveInitialWindowBounds(
Option.some({ x: 5000, y: 5000, width: 800, height: 600, maximized: true }),
DISPLAYS,
DEFAULTS,
),
{ bounds: { width: 800, height: 600 }, maximize: true },
);
});

it("uses size only when the saved state has no position", () => {
assert.deepStrictEqual(
DesktopWindowState.resolveInitialWindowBounds(
Option.some({ width: 800, height: 600 }),
DISPLAYS,
DEFAULTS,
),
{ bounds: { width: 800, height: 600 }, maximize: false },
);
});
});
130 changes: 130 additions & 0 deletions apps/desktop/src/window/DesktopWindowState.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
import { fromLenientJson } from "@t3tools/shared/schemaJson";
import * as Context from "effect/Context";
import * as Effect from "effect/Effect";
import * as FileSystem from "effect/FileSystem";
import * as Layer from "effect/Layer";
import * as Option from "effect/Option";
import * as Path from "effect/Path";
import * as Schema from "effect/Schema";

import * as DesktopEnvironment from "../app/DesktopEnvironment.ts";

// Persisted geometry of the main window. `width`/`height` are the un-maximized
// ("normal") content bounds; `maximized` restores the maximized state on top of
// them. `x`/`y` are optional so a document written before we tracked position
// (or one whose position is off-screen on load) still restores the size.
const WindowStateDocument = Schema.Struct({
x: Schema.optionalKey(Schema.Number),
y: Schema.optionalKey(Schema.Number),
width: Schema.Number,
height: Schema.Number,
maximized: Schema.optionalKey(Schema.Boolean),
});

export type WindowState = typeof WindowStateDocument.Type;

const WindowStateJson = fromLenientJson(WindowStateDocument);
const decodeWindowStateJson = Schema.decodeEffect(WindowStateJson);
const encodeWindowStateJson = Schema.encodeEffect(WindowStateJson);

export class DesktopWindowStateWriteError extends Schema.TaggedErrorClass<DesktopWindowStateWriteError>()(
"DesktopWindowStateWriteError",
{
path: Schema.String,
cause: Schema.Defect(),
},
) {
override get message(): string {
return `Failed to persist desktop window state at ${this.path}.`;
}
}

export interface WindowRect {
readonly x: number;
readonly y: number;
readonly width: number;
readonly height: number;
}

export interface InitialWindowBounds {
// Spread straight into BrowserWindow options. `x`/`y` are dropped when the
// saved position lands outside every display so the window can't open
// off-screen (e.g. after an external monitor is unplugged).
readonly bounds: { x?: number; y?: number; width: number; height: number };
readonly maximize: boolean;
}

const isUsableSize = (state: WindowState): boolean =>
Number.isFinite(state.width) &&
state.width > 0 &&
Number.isFinite(state.height) &&
state.height > 0;

const rectsIntersect = (a: WindowRect, b: WindowRect): boolean =>
a.x < b.x + b.width && a.x + a.width > b.x && a.y < b.y + b.height && a.y + a.height > b.y;

// Pure so it can be unit-tested without Electron. `displays` are the work areas
// of the connected screens; pass `screen.getAllDisplays().map((d) => d.workArea)`.
export function resolveInitialWindowBounds(
saved: Option.Option<WindowState>,
displays: readonly WindowRect[],
defaults: { readonly width: number; readonly height: number },
): InitialWindowBounds {
if (Option.isNone(saved)) {
return { bounds: { ...defaults }, maximize: false };
}

const state = saved.value;
const size = { width: state.width, height: state.height };
const maximize = state.maximized === true;

if (state.x === undefined || state.y === undefined) {
return { bounds: size, maximize };
}

const rect: WindowRect = { x: state.x, y: state.y, width: state.width, height: state.height };
const onScreen = displays.some((display) => rectsIntersect(display, rect));

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.

🟡 Medium window/DesktopWindowState.ts:86

resolveInitialWindowBounds only requires any pixel of overlap with a display via rectsIntersect. A saved state like { x: 100, y: -590, width: 800, height: 600 } still intersects the screen by a few pixels, so this code restores the off-screen x/y instead of dropping them. Because the app uses a hidden title bar, this can reopen the window with its draggable area completely off-screen and effectively inaccessible, even though the PR is meant to prevent off-screen restores. Consider requiring the title-bar/caption region (or a larger minimum visible margin) to intersect a display before restoring x/y.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/desktop/src/window/DesktopWindowState.ts around line 86:

`resolveInitialWindowBounds` only requires any pixel of overlap with a display via `rectsIntersect`. A saved state like `{ x: 100, y: -590, width: 800, height: 600 }` still intersects the screen by a few pixels, so this code restores the off-screen `x`/`y` instead of dropping them. Because the app uses a hidden title bar, this can reopen the window with its draggable area completely off-screen and effectively inaccessible, even though the PR is meant to prevent off-screen restores. Consider requiring the title-bar/caption region (or a larger minimum visible margin) to intersect a display before restoring `x`/`y`.

return onScreen ? { bounds: rect, maximize } : { bounds: size, maximize };
}

export class DesktopWindowState extends Context.Service<
DesktopWindowState,
{
readonly load: Effect.Effect<Option.Option<WindowState>>;
readonly save: (state: WindowState) => Effect.Effect<void, DesktopWindowStateWriteError>;
}
>()("@t3tools/desktop/window/DesktopWindowState") {}

export const make = Effect.gen(function* () {
const environment = yield* DesktopEnvironment.DesktopEnvironment;
const fileSystem = yield* FileSystem.FileSystem;
const path = yield* Path.Path;

const load = fileSystem.readFileString(environment.windowStatePath).pipe(
Effect.flatMap(decodeWindowStateJson),
Effect.map((state) => (isUsableSize(state) ? Option.some(state) : Option.none<WindowState>())),
// Missing file, unreadable file, or a malformed document all fall back to
// "no saved state" so a first launch (or a corrupt file) opens at defaults.
Effect.orElseSucceed(() => Option.none<WindowState>()),
Effect.withSpan("desktop.windowState.load"),
);

const save = (state: WindowState) =>
encodeWindowStateJson(state).pipe(
Effect.flatMap((encoded) =>
fileSystem
.makeDirectory(path.dirname(environment.windowStatePath), { recursive: true })
.pipe(
Effect.andThen(fileSystem.writeFileString(environment.windowStatePath, `${encoded}\n`)),
),
),
Effect.mapError(
(cause) => new DesktopWindowStateWriteError({ path: environment.windowStatePath, cause }),
),
Effect.withSpan("desktop.windowState.save"),
);

return DesktopWindowState.of({ load, save });
});

export const layer = Layer.effect(DesktopWindowState, make);
Loading