Skip to content
Merged
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
733 changes: 440 additions & 293 deletions .github/workflows/release.yml

Large diffs are not rendered by default.

8 changes: 7 additions & 1 deletion apps/desktop/src/app/DesktopLifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,14 @@ function addScopedListener<Args extends ReadonlyArray<unknown>>(
}

const requestDesktopShutdownAndWait = Effect.fn("desktop.lifecycle.requestShutdownAndWait")(
function* (): Effect.fn.Return<void, never, DesktopShutdown.DesktopShutdown> {
function* (): Effect.fn.Return<
void,
never,
DesktopShutdown.DesktopShutdown | DesktopWindow.DesktopWindow
> {
const shutdown = yield* DesktopShutdown.DesktopShutdown;
const desktopWindow = yield* DesktopWindow.DesktopWindow;
yield* desktopWindow.flushMainWindowBounds;
yield* shutdown.request;
yield* shutdown.awaitComplete;
Comment on lines 81 to 85

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Bound the bounds flush so a stalled settings write cannot block quit/relaunch.

flushMainWindowBounds joins an in-flight settings file write. handleBeforeQuit already called event.preventDefault(), so if that write stalls the app never reaches shutdown.request and never quits. Persisting window geometry is best-effort and should not gate shutdown.

As per coding guidelines, prioritize "reliability, predictable behavior under load and failures".

🛡️ Proposed fix
     const shutdown = yield* DesktopShutdown.DesktopShutdown;
     const desktopWindow = yield* DesktopWindow.DesktopWindow;
-    yield* desktopWindow.flushMainWindowBounds;
+    yield* desktopWindow.flushMainWindowBounds.pipe(
+      Effect.timeout(MAIN_WINDOW_BOUNDS_FLUSH_TIMEOUT_MS),
+      Effect.ignore,
+    );
     yield* shutdown.request;
📝 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
const shutdown = yield* DesktopShutdown.DesktopShutdown;
const desktopWindow = yield* DesktopWindow.DesktopWindow;
yield* desktopWindow.flushMainWindowBounds;
yield* shutdown.request;
yield* shutdown.awaitComplete;
const shutdown = yield* DesktopShutdown.DesktopShutdown;
const desktopWindow = yield* DesktopWindow.DesktopWindow;
yield* desktopWindow.flushMainWindowBounds.pipe(
Effect.timeout(MAIN_WINDOW_BOUNDS_FLUSH_TIMEOUT_MS),
Effect.ignore,
);
yield* shutdown.request;
yield* shutdown.awaitComplete;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/app/DesktopLifecycle.ts` around lines 81 - 85, Update the
shutdown flow in handleBeforeQuit so DesktopWindow.flushMainWindowBounds is
best-effort and bounded, ensuring a stalled settings write cannot delay
shutdown.request or shutdown.awaitComplete. Add the established timeout/fallback
behavior around the flush, then always continue to the shutdown steps after
success, failure, or timeout.

Source: Coding guidelines

},
Expand Down
1 change: 1 addition & 0 deletions apps/desktop/src/backend/DesktopBackendPool.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ function makePoolLayer(
showConnectingSplash: Effect.void,
handleBackendReady: () => Effect.void,
handleBackendNotReady: Effect.void,
flushMainWindowBounds: Effect.void,
dispatchMenuAction: () => Effect.die("unexpected menu action"),
syncAppearance: Effect.void,
} satisfies DesktopWindow.DesktopWindow["Service"]),
Expand Down
1 change: 1 addition & 0 deletions apps/desktop/src/backend/DesktopServerExposure.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ describe("DesktopServerExposure", () => {
const settingsLayer = Layer.succeed(DesktopAppSettings.DesktopAppSettings, {
get: Effect.succeed(DesktopAppSettings.DEFAULT_DESKTOP_SETTINGS),
load: Effect.succeed(DesktopAppSettings.DEFAULT_DESKTOP_SETTINGS),
setMainWindowBounds: () => Effect.die("unexpected main window bounds update"),
setServerExposureMode: () => Effect.fail(settingsFailure),
setTailscaleServe: () => Effect.fail(settingsFailure),
setUpdateChannel: () => Effect.die("unexpected update channel change"),
Expand Down
45 changes: 45 additions & 0 deletions apps/desktop/src/settings/DesktopAppSettings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,17 @@ import * as DesktopEnvironment from "../app/DesktopEnvironment.ts";
import * as DesktopAppSettings from "./DesktopAppSettings.ts";

const DesktopSettingsPatch = Schema.Struct({
mainWindowBounds: Schema.optionalKey(
Schema.NullOr(
Schema.Struct({
x: Schema.Number,
y: Schema.Number,
width: Schema.Number,
height: Schema.Number,
}),
),
),
mainWindowMaximized: Schema.optionalKey(Schema.Boolean),
serverExposureMode: Schema.optionalKey(Schema.Literals(["local-only", "network-accessible"])),
tailscaleServeEnabled: Schema.optionalKey(Schema.Boolean),
tailscaleServePort: Schema.optionalKey(Schema.Number),
Expand Down Expand Up @@ -91,6 +102,8 @@ describe("DesktopSettings", () => {
assert.deepEqual(
DesktopAppSettings.resolveDefaultDesktopSettings("0.0.17-nightly.20260415.1"),
{
mainWindowBounds: null,
mainWindowMaximized: false,
serverExposureMode: "local-only",
tailscaleServeEnabled: false,
tailscaleServePort: 443,
Expand All @@ -116,6 +129,8 @@ describe("DesktopSettings", () => {
});

assert.deepEqual(yield* settings.load, {
mainWindowBounds: null,
mainWindowMaximized: false,
serverExposureMode: "network-accessible",
tailscaleServeEnabled: true,
tailscaleServePort: 8443,
Expand Down Expand Up @@ -215,10 +230,13 @@ describe("DesktopSettings", () => {
"serverExposureMode": "network-accessible",
"tailscaleServeEnabled": true,
"tailscaleServePort": 8443,
"mainWindowBounds": { "x": 120, "y": 80, "width": 1280, "height": 900 },
}\n`,
);

assert.deepEqual(yield* settings.load, {
mainWindowBounds: { x: 120, y: 80, width: 1280, height: 900 },
mainWindowMaximized: false,
serverExposureMode: "network-accessible",
tailscaleServeEnabled: true,
tailscaleServePort: 8443,
Expand All @@ -232,19 +250,40 @@ describe("DesktopSettings", () => {
),
);

it.effect("rejects window bounds that do not satisfy the domain schema", () =>
withSettings(
Effect.gen(function* () {
const settings = yield* DesktopAppSettings.DesktopAppSettings;
yield* writeSettingsPatch({
mainWindowBounds: { x: 10.5, y: 20, width: 839, height: 620 },
mainWindowMaximized: true,
serverExposureMode: "network-accessible",
});

const loaded = yield* settings.load;
assert.isNull(loaded.mainWindowBounds);
assert.isFalse(loaded.mainWindowMaximized);
assert.equal(loaded.serverExposureMode, "network-accessible");
}),
),
);

it.effect("persists sparse desktop settings documents", () =>
withSettings(
Effect.gen(function* () {
const environment = yield* DesktopEnvironment.DesktopEnvironment;
const fileSystem = yield* FileSystem.FileSystem;
const settings = yield* DesktopAppSettings.DesktopAppSettings;

yield* settings.setMainWindowBounds({ x: -1200, y: 40, width: 1440, height: 960 }, true);
yield* settings.setServerExposureMode("network-accessible");

const persisted = yield* decodeDesktopSettingsPatch(
yield* fileSystem.readFileString(environment.desktopSettingsPath),
);
assert.deepEqual(persisted, {
mainWindowBounds: { x: -1200, y: 40, width: 1440, height: 960 },
mainWindowMaximized: true,
serverExposureMode: "network-accessible",
} satisfies typeof DesktopSettingsPatch.Type);
}),
Expand All @@ -261,6 +300,8 @@ describe("DesktopSettings", () => {
});

assert.deepEqual(yield* settings.load, {
mainWindowBounds: null,
mainWindowMaximized: false,
serverExposureMode: "local-only",
tailscaleServeEnabled: false,
tailscaleServePort: 443,
Expand All @@ -286,6 +327,8 @@ describe("DesktopSettings", () => {
});

assert.deepEqual(yield* settings.load, {
mainWindowBounds: null,
mainWindowMaximized: false,
serverExposureMode: "local-only",
tailscaleServeEnabled: false,
tailscaleServePort: 443,
Expand All @@ -310,6 +353,8 @@ describe("DesktopSettings", () => {
});

assert.deepEqual(yield* settings.load, {
mainWindowBounds: null,
mainWindowMaximized: false,
serverExposureMode: "local-only",
tailscaleServeEnabled: true,
tailscaleServePort: 443,
Expand Down
77 changes: 77 additions & 0 deletions apps/desktop/src/settings/DesktopAppSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import { resolveDefaultDesktopUpdateChannel } from "../updates/updateChannels.ts
import { isValidDistroName } from "../wsl/wslPathParsing.ts";

export interface DesktopSettings {
readonly mainWindowBounds: DesktopWindowBounds | null;
readonly mainWindowMaximized: boolean;
readonly serverExposureMode: DesktopServerExposureMode;
readonly tailscaleServeEnabled: boolean;
readonly tailscaleServePort: number;
Expand Down Expand Up @@ -48,8 +50,25 @@ export interface DesktopSettingsChange {
}

export const DEFAULT_TAILSCALE_SERVE_PORT = 443;
const MIN_MAIN_WINDOW_SIZE = {
width: 840,
height: 620,
} as const;
export const DesktopWindowBoundsSchema = Schema.Struct({
x: Schema.Int,
y: Schema.Int,
width: Schema.Int.check(Schema.isGreaterThanOrEqualTo(MIN_MAIN_WINDOW_SIZE.width)),
height: Schema.Int.check(Schema.isGreaterThanOrEqualTo(MIN_MAIN_WINDOW_SIZE.height)),
});
export type DesktopWindowBounds = typeof DesktopWindowBoundsSchema.Type;
export const DEFAULT_MAIN_WINDOW_SIZE = {
width: 1100,
height: 780,
} as const;

export const DEFAULT_DESKTOP_SETTINGS: DesktopSettings = {
mainWindowBounds: null,
mainWindowMaximized: false,
serverExposureMode: "local-only",
tailscaleServeEnabled: false,
tailscaleServePort: DEFAULT_TAILSCALE_SERVE_PORT,
Expand All @@ -60,7 +79,16 @@ export const DEFAULT_DESKTOP_SETTINGS: DesktopSettings = {
wslOnly: false,
};

const DesktopWindowBoundsDocument = Schema.Struct({
x: Schema.Number,
y: Schema.Number,
width: Schema.Number,
height: Schema.Number,
});

const DesktopSettingsDocument = Schema.Struct({
mainWindowBounds: Schema.optionalKey(Schema.NullOr(DesktopWindowBoundsDocument)),
mainWindowMaximized: Schema.optionalKey(Schema.Boolean),
serverExposureMode: Schema.optionalKey(DesktopServerExposureModeSchema),
tailscaleServeEnabled: Schema.optionalKey(Schema.Boolean),
tailscaleServePort: Schema.optionalKey(Schema.Number),
Expand All @@ -81,6 +109,8 @@ type Mutable<T> = { -readonly [K in keyof T]: T[K] };
const DesktopSettingsJson = fromLenientJson(DesktopSettingsDocument);
const decodeDesktopSettingsJson = Schema.decodeEffect(DesktopSettingsJson);
const encodeDesktopSettingsJson = Schema.encodeEffect(DesktopSettingsJson);
const decodeDesktopWindowBounds = Schema.decodeUnknownOption(DesktopWindowBoundsSchema);
const desktopWindowBoundsEquivalence = Schema.toEquivalence(DesktopWindowBoundsSchema);

const settingsChange = (settings: DesktopSettings, changed: boolean): DesktopSettingsChange => ({
settings,
Expand Down Expand Up @@ -114,6 +144,10 @@ export class DesktopAppSettings extends Context.Service<
{
readonly load: Effect.Effect<DesktopSettings>;
readonly get: Effect.Effect<DesktopSettings>;
readonly setMainWindowBounds: (
bounds: DesktopWindowBounds,
isMaximized: boolean,
) => Effect.Effect<DesktopSettingsChange, DesktopSettingsWriteError>;
readonly setServerExposureMode: (
mode: DesktopServerExposureMode,
) => Effect.Effect<DesktopSettingsChange, DesktopSettingsWriteError>;
Expand Down Expand Up @@ -158,11 +192,16 @@ function normalizeWslDistro(value: unknown): string | null {
return typeof value === "string" && isValidDistroName(value) ? value : null;
}

export function normalizeMainWindowBounds(value: unknown): DesktopWindowBounds | null {
return Option.getOrNull(decodeDesktopWindowBounds(value));
}

function normalizeDesktopSettingsDocument(
parsed: DesktopSettingsDocument,
appVersion: string,
): DesktopSettings {
const defaultSettings = resolveDefaultDesktopSettings(appVersion);
const mainWindowBounds = normalizeMainWindowBounds(parsed.mainWindowBounds);
const parsedUpdateChannel = Option.fromNullishOr(parsed.updateChannel);
const isLegacySettings = parsed.updateChannelConfiguredByUser === undefined;
const updateChannelConfiguredByUser =
Expand All @@ -177,6 +216,8 @@ function normalizeDesktopSettingsDocument(
(parsed.wslBackendEnabled === undefined && parsed.wslMode === "wsl");

return {
mainWindowBounds,
mainWindowMaximized: mainWindowBounds !== null && parsed.mainWindowMaximized === true,
serverExposureMode:
parsed.serverExposureMode === "network-accessible" ? "network-accessible" : "local-only",
tailscaleServeEnabled: parsed.tailscaleServeEnabled === true,
Expand All @@ -197,6 +238,12 @@ function toDesktopSettingsDocument(
): DesktopSettingsDocument {
const document: Mutable<DesktopSettingsDocument> = {};

if (settings.mainWindowBounds !== null) {
document.mainWindowBounds = settings.mainWindowBounds;
}
if (settings.mainWindowMaximized) {
document.mainWindowMaximized = true;
}
if (settings.serverExposureMode !== defaults.serverExposureMode) {
document.serverExposureMode = settings.serverExposureMode;
}
Expand Down Expand Up @@ -237,6 +284,22 @@ function setServerExposureMode(
};
}

function setMainWindowBounds(
settings: DesktopSettings,
bounds: DesktopWindowBounds,
isMaximized: boolean,
): DesktopSettings {
return settings.mainWindowBounds !== null &&
desktopWindowBoundsEquivalence(settings.mainWindowBounds, bounds) &&
settings.mainWindowMaximized === isMaximized
? settings
: {
...settings,
mainWindowBounds: bounds,
mainWindowMaximized: isMaximized,
};
}

function setTailscaleServe(
settings: DesktopSettings,
input: { readonly enabled: boolean; readonly port: Option.Option<number> },
Expand Down Expand Up @@ -431,6 +494,18 @@ export const make = Effect.gen(function* () {
);
return yield* SynchronizedRef.setAndGet(settingsRef, settings);
}).pipe(Effect.withSpan("desktop.settings.load")),
setMainWindowBounds: (bounds, isMaximized) =>
persist((settings) => setMainWindowBounds(settings, bounds, isMaximized)).pipe(
Effect.withSpan("desktop.settings.setMainWindowBounds", {
attributes: {
x: bounds.x,
y: bounds.y,
width: bounds.width,
height: bounds.height,
isMaximized,
},
}),
),
setServerExposureMode: (mode) =>
persist((settings) => setServerExposureMode(settings, mode)).pipe(
Effect.withSpan("desktop.settings.setServerExposureMode", { attributes: { mode } }),
Expand Down Expand Up @@ -488,6 +563,8 @@ export const layerTest = (initialSettings: DesktopSettings = DEFAULT_DESKTOP_SET
return DesktopAppSettings.of({
get: SynchronizedRef.get(settingsRef),
load: SynchronizedRef.get(settingsRef),
setMainWindowBounds: (bounds, isMaximized) =>
update((settings) => setMainWindowBounds(settings, bounds, isMaximized)),
setServerExposureMode: (mode) =>
update((settings) => setServerExposureMode(settings, mode)),
setTailscaleServe: (input) => update((settings) => setTailscaleServe(settings, input)),
Expand Down
1 change: 1 addition & 0 deletions apps/desktop/src/updates/DesktopUpdates.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ function makeHarness(options: UpdatesHarnessOptions = {}) {
? Layer.succeed(DesktopAppSettings.DesktopAppSettings, {
get: Effect.succeed(DesktopAppSettings.DEFAULT_DESKTOP_SETTINGS),
load: Effect.succeed(DesktopAppSettings.DEFAULT_DESKTOP_SETTINGS),
setMainWindowBounds: () => Effect.die("unexpected main window bounds update"),
setServerExposureMode: () => Effect.die("unexpected server exposure update"),
setTailscaleServe: () => Effect.die("unexpected Tailscale Serve update"),
setUpdateChannel: () => Effect.fail(setUpdateChannelError),
Expand Down
1 change: 1 addition & 0 deletions apps/desktop/src/window/DesktopApplicationMenu.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ const makeDesktopWindowLayer = (selectedAction: Deferred.Deferred<string>) =>
showConnectingSplash: Effect.void,
handleBackendReady: () => Effect.void,
handleBackendNotReady: Effect.void,
flushMainWindowBounds: Effect.void,
dispatchMenuAction: (action) => Deferred.succeed(selectedAction, action).pipe(Effect.asVoid),
syncAppearance: Effect.void,
} satisfies DesktopWindow.DesktopWindow["Service"]);
Expand Down
Loading
Loading