From 1a54ca6848088af88d890fa9c58f47df18820dec Mon Sep 17 00:00:00 2001 From: Luxe7 <137878909+Luxe7@users.noreply.github.com> Date: Tue, 3 Nov 2026 21:38:59 +0800 Subject: [PATCH 1/3] fix(desktop): keep closing when window state cannot be saved Catch window-state filesystem errors so later closed listeners still run. Co-authored-by: Codex --- packages/desktop/src/main/windows/window-state.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/desktop/src/main/windows/window-state.ts b/packages/desktop/src/main/windows/window-state.ts index cce428c6cbe5..10a7840e2b7b 100644 --- a/packages/desktop/src/main/windows/window-state.ts +++ b/packages/desktop/src/main/windows/window-state.ts @@ -137,9 +137,14 @@ export function manageWindowState(win: BrowserWindow, file: string, initial: Win win.off("move", changed) win.off("close", update) win.off("closed", closed) - mkdirSync(path.dirname(file), { recursive: true }) - writeFileSync(`${file}.tmp`, JSON.stringify(state)) - renameSync(`${file}.tmp`, file) + + try { + mkdirSync(path.dirname(file), { recursive: true }) + writeFileSync(`${file}.tmp`, JSON.stringify(state)) + renameSync(`${file}.tmp`, file) + } catch (error) { + console.warn("Failed to save window state", { file, error }) + } } win.on("resize", changed) From ba1302c46a75cdc6c769e6fa348478a54c2759d4 Mon Sep 17 00:00:00 2001 From: Luxe7 <137878909+Luxe7@users.noreply.github.com> Date: Tue, 3 Nov 2026 21:39:17 +0800 Subject: [PATCH 2/3] test(desktop): cover window state save failures Exercise real filesystem failures and verify later close listeners still run. Co-authored-by: Codex --- .../src/main/windows/window-state.test.ts | 85 ++++++++++++++++++- 1 file changed, 82 insertions(+), 3 deletions(-) diff --git a/packages/desktop/src/main/windows/window-state.test.ts b/packages/desktop/src/main/windows/window-state.test.ts index 295b690385c6..3f8a90ee4146 100644 --- a/packages/desktop/src/main/windows/window-state.test.ts +++ b/packages/desktop/src/main/windows/window-state.test.ts @@ -1,8 +1,10 @@ -import { describe, expect, test } from "bun:test" -import { mkdtempSync, writeFileSync } from "node:fs" +import { describe, expect, spyOn, test } from "bun:test" +import { EventEmitter } from "node:events" +import type { BrowserWindow } from "electron" +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs" import { tmpdir } from "node:os" import path from "node:path" -import { readWindowState, resolveWindowState } from "./window-state" +import { manageWindowState, readWindowState, resolveWindowState } from "./window-state" const defaults = { width: 1280, height: 800 } @@ -109,3 +111,80 @@ describe("window state", () => { expect(readWindowState(path.join(dir, "missing.json"))).toBeUndefined() }) }) + +describe("saving window state on close", () => { + test.each(["success", "mkdir", "write", "rename", ...(process.platform === "linux" ? ["full"] : [])])( + "continues closing after %s", + (stage) => { + const dir = mkdtempSync(path.join(tmpdir(), "opencode-window-close-")) + const file = path.join(dir, "state", "window.json") + const warn = spyOn(console, "warn").mockImplementation(() => {}) + const bounds = { x: 10, y: 20, width: 800, height: 600 } + + const win = Object.assign(new EventEmitter(), { + isDestroyed: () => false, + getBounds: () => bounds, + isMaximized: () => false, + isMinimized: () => false, + isFullScreen: () => false, + }) + + try { + if (stage === "mkdir") writeFileSync(path.dirname(file), "blocked") + + if (stage === "write") { + mkdirSync(path.dirname(file)) + writeFileSync(file, "previous state") + mkdirSync(`${file}.tmp`) + } + + if (stage === "rename") mkdirSync(file, { recursive: true }) + + // /dev/full provides a real ENOSPC without filling the machine's disk. + if (stage === "full") { + mkdirSync(path.dirname(file)) + symlinkSync("/dev/full", `${file}.tmp`) + } + + // SAFETY: this fixture implements the window events and methods used for this non-maximized state. + manageWindowState(win as BrowserWindow, file, defaults, displays) + const closed = { called: false } + win.on("closed", () => { + closed.called = true + }) + win.emit("close") + expect(() => win.emit("closed")).not.toThrow() + expect(closed.called).toBe(true) + expect(win.listenerCount("resize")).toBe(0) + expect(win.listenerCount("move")).toBe(0) + expect(win.listenerCount("close")).toBe(0) + + if (stage === "success") { + expect(JSON.parse(readFileSync(file, "utf8"))).toEqual({ + ...bounds, + isMaximized: false, + isFullScreen: false, + displayBounds: primary, + }) + expect(existsSync(`${file}.tmp`)).toBe(false) + expect(warn).not.toHaveBeenCalled() + + return + } + + expect(warn).toHaveBeenCalledWith("Failed to save window state", { file, error: expect.any(Error) }) + + if (stage === "write") expect(readFileSync(file, "utf8")).toBe("previous state") + + if (stage === "full") + expect(warn).toHaveBeenCalledWith("Failed to save window state", { + file, + error: expect.objectContaining({ code: "ENOSPC" }), + }) + } finally { + warn.mockRestore() + rmSync(dir, { recursive: true, force: true }) + } + }, + ) +}) From e35b64b53dc12df0591e69e0daffb78792bdbd6a Mon Sep 17 00:00:00 2001 From: Luxe7 <137878909+Luxe7@users.noreply.github.com> Date: Wed, 4 Nov 2026 12:44:17 +0800 Subject: [PATCH 3/3] fix(desktop): route window save failures through desktop logging Report save errors through the caller and upgrade early-window logging on adoption. Keep one focused filesystem regression test. Co-authored-by: Codex --- packages/desktop/src/main/windows/early.ts | 22 ++-- packages/desktop/src/main/windows/index.ts | 15 ++- .../src/main/windows/window-state.test.ts | 107 +++++------------- .../desktop/src/main/windows/window-state.ts | 10 +- 4 files changed, 59 insertions(+), 95 deletions(-) diff --git a/packages/desktop/src/main/windows/early.ts b/packages/desktop/src/main/windows/early.ts index 99b750ca65df..ac0a416872f8 100644 --- a/packages/desktop/src/main/windows/early.ts +++ b/packages/desktop/src/main/windows/early.ts @@ -26,6 +26,7 @@ export type EarlyWindow = { shownAt: number // Navigation policy is wired before the layers exist; the adopter swaps in the logged version. openExternal: (url: string) => void + reportStateError: (error: Error) => void } let pending: EarlyWindow | undefined @@ -75,21 +76,12 @@ export function createEarlyWindow() { }, }) - manageWindowState(win, file, state, displays) - - if (deferReveal) win.show() - // Closing the only window before the rest of the app has adopted it is a quit. - win.once("closed", () => { - if (pending?.win !== win) return - pending = undefined - app.quit() - }) - const record: EarlyWindow = { id, win, state, shownAt: Date.now(), + reportStateError: (error) => console.warn("failed to save window state", { file, error }), openExternal: (url) => { const target = resolveExternalURL(url) @@ -97,6 +89,16 @@ export function createEarlyWindow() { }, } + manageWindowState(win, file, state, displays, (error) => record.reportStateError(error)) + + if (deferReveal) win.show() + // Closing the only window before the rest of the app has adopted it is a quit. + win.once("closed", () => { + if (pending?.win !== win) return + pending = undefined + app.quit() + }) + pending = record // The renderer boots while the main bundle and layers load, instead of after them. Everything the // page needs before its first request is wired here; the IPC port arrives once the layers are up. diff --git a/packages/desktop/src/main/windows/index.ts b/packages/desktop/src/main/windows/index.ts index 892603f5a1c7..7005982ccfee 100644 --- a/packages/desktop/src/main/windows/index.ts +++ b/packages/desktop/src/main/windows/index.ts @@ -159,15 +159,20 @@ export const mainWindows = Effect.fn("Window.make")(function* () { }, }) - // The early window was secured and loaded when it was created; only its external-URL policy is - // upgraded to the logged one. - if (early) early.openExternal = (url) => runFork(openExternalURL(url)) + const reportStateError = (error: Error) => + runFork(scoped("window", Effect.logWarning("failed to save window state", { file: stateFile, error }))) + + // Upgrade the early window's callbacks once the logging layer is available. + if (early) { + early.openExternal = (url) => runFork(openExternalURL(url)) + early.reportStateError = reportStateError + } if (!early) { allowRendererPermissions(win) wireNavigationPolicy(win, (url) => runFork(openExternalURL(url))) wireRendererHeaders(win) - manageWindowState(win, stateFile, state, displays) + manageWindowState(win, stateFile, state, displays, reportStateError) } wireWindowRecovery(win, id, () => relaunchHandler()) @@ -239,5 +244,3 @@ export const mainWindows = Effect.fn("Window.make")(function* () { return { create, restore } }) - - diff --git a/packages/desktop/src/main/windows/window-state.test.ts b/packages/desktop/src/main/windows/window-state.test.ts index 3f8a90ee4146..7efc8f961f33 100644 --- a/packages/desktop/src/main/windows/window-state.test.ts +++ b/packages/desktop/src/main/windows/window-state.test.ts @@ -1,7 +1,7 @@ -import { describe, expect, spyOn, test } from "bun:test" +import { describe, expect, mock, test } from "bun:test" import { EventEmitter } from "node:events" import type { BrowserWindow } from "electron" -import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs" +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs" import { tmpdir } from "node:os" import path from "node:path" import { manageWindowState, readWindowState, resolveWindowState } from "./window-state" @@ -112,79 +112,32 @@ describe("window state", () => { }) }) -describe("saving window state on close", () => { - test.each(["success", "mkdir", "write", "rename", ...(process.platform === "linux" ? ["full"] : [])])( - "continues closing after %s", - (stage) => { - const dir = mkdtempSync(path.join(tmpdir(), "opencode-window-close-")) - const file = path.join(dir, "state", "window.json") - const warn = spyOn(console, "warn").mockImplementation(() => {}) - const bounds = { x: 10, y: 20, width: 800, height: 600 } - - const win = Object.assign(new EventEmitter(), { - isDestroyed: () => false, - getBounds: () => bounds, - isMaximized: () => false, - isMinimized: () => false, - isFullScreen: () => false, - }) - - try { - if (stage === "mkdir") writeFileSync(path.dirname(file), "blocked") - - if (stage === "write") { - mkdirSync(path.dirname(file)) - writeFileSync(file, "previous state") - mkdirSync(`${file}.tmp`) - } - - if (stage === "rename") mkdirSync(file, { recursive: true }) - - // /dev/full provides a real ENOSPC without filling the machine's disk. - if (stage === "full") { - mkdirSync(path.dirname(file)) - symlinkSync("/dev/full", `${file}.tmp`) - } - - // SAFETY: this fixture implements the window events and methods used for this non-maximized state. - manageWindowState(win as BrowserWindow, file, defaults, displays) - const closed = { called: false } - win.on("closed", () => { - closed.called = true - }) - win.emit("close") - expect(() => win.emit("closed")).not.toThrow() - expect(closed.called).toBe(true) - expect(win.listenerCount("resize")).toBe(0) - expect(win.listenerCount("move")).toBe(0) - expect(win.listenerCount("close")).toBe(0) - - if (stage === "success") { - expect(JSON.parse(readFileSync(file, "utf8"))).toEqual({ - ...bounds, - isMaximized: false, - isFullScreen: false, - displayBounds: primary, - }) - expect(existsSync(`${file}.tmp`)).toBe(false) - expect(warn).not.toHaveBeenCalled() - - return - } - - expect(warn).toHaveBeenCalledWith("Failed to save window state", { file, error: expect.any(Error) }) - - if (stage === "write") expect(readFileSync(file, "utf8")).toBe("previous state") - - if (stage === "full") - expect(warn).toHaveBeenCalledWith("Failed to save window state", { - file, - error: expect.objectContaining({ code: "ENOSPC" }), - }) - } finally { - warn.mockRestore() - rmSync(dir, { recursive: true, force: true }) - } - }, - ) +test("a failed window-state save reports the error and allows closing to continue", () => { + const dir = mkdtempSync(path.join(tmpdir(), "opencode-window-close-")) + const file = path.join(dir, "window.json") + const report = mock((_error: Error) => {}) + const closed = mock(() => {}) + + const win = Object.assign(new EventEmitter(), { + isDestroyed: () => false, + getBounds: () => ({ x: 10, y: 20, width: 800, height: 600 }), + isMaximized: () => false, + isMinimized: () => false, + isFullScreen: () => false, + }) + + try { + writeFileSync(file, "previous state") + mkdirSync(`${file}.tmp`) + // SAFETY: the fixture implements the window events and getters used for this non-maximized state. + manageWindowState(win as BrowserWindow, file, defaults, displays, report) + win.on("closed", closed) + win.emit("close") + expect(() => win.emit("closed")).not.toThrow() + expect(closed).toHaveBeenCalledTimes(1) + expect(report).toHaveBeenCalledWith(expect.any(Error)) + expect(readFileSync(file, "utf8")).toBe("previous state") + } finally { + rmSync(dir, { recursive: true, force: true }) + } }) diff --git a/packages/desktop/src/main/windows/window-state.ts b/packages/desktop/src/main/windows/window-state.ts index 10a7840e2b7b..4d8cbb76fca9 100644 --- a/packages/desktop/src/main/windows/window-state.ts +++ b/packages/desktop/src/main/windows/window-state.ts @@ -102,7 +102,13 @@ export function readWindowState(file: string) { // Tracks the window until it closes and writes the final state then. Bounds are recorded while // the window is in its normal state so a maximized window restores to its previous size. -export function manageWindowState(win: BrowserWindow, file: string, initial: WindowState, displays: Displays) { +export function manageWindowState( + win: BrowserWindow, + file: string, + initial: WindowState, + displays: Displays, + onError: (error: Error) => void, +) { const state = { ...initial } if (state.isMaximized) win.maximize() @@ -143,7 +149,7 @@ export function manageWindowState(win: BrowserWindow, file: string, initial: Win writeFileSync(`${file}.tmp`, JSON.stringify(state)) renameSync(`${file}.tmp`, file) } catch (error) { - console.warn("Failed to save window state", { file, error }) + onError(error instanceof Error ? error : new Error(String(error))) } }