fix(desktop): persist zoom across app restarts - #56802

The following comment was made by an LLM, it may be inaccurate:

Opened from the rebased branch discussed in #50169, now that it applies cleanly to dev.

• @mitivil: this is your change replayed on today's dev with authorship and the commit message preserved (46f7afcfd). If you'd rather own it here, feel free to push the same commit to persist-window-zoom or take over this PR — whichever is less work for you.
• @wc4440222: there is now a mergeable PR instead of just a branch. If CI publishes an artifact (or anyone builds), the Windows verification you offered would be valuable; the report is already confirmed on macOS arm64 and Linux.

The only remaining blocker is a code-owner review — everything else is green (non-draft, bot checks pass, small self-contained change). This supersedes #50169.

Thanks for updating your PR! It now meets our contributing guidelines. 👍

Adding the Windows verification side of this from #50169, since the bot review above couldn't launch the desktop app anywhere.

• Platform: Windows 11, Desktop 2.0.32
• I'm the Windows 11 / 2.0.32 row in the cross-platform table in fix(desktop): persist zoom across app restarts #50169, so I can do a real before/after on a mergeable build
• @darknose — if CI publishes an artifact for this branch, or you push a temporary build somewhere, I'll install it and report back with the exact version string and what zoom now does across a full quit/relaunch

On the appearance.ts review note about writeFileSync + renameSync on every change: I can specifically exercise the Windows EPERM case, since that's the platform the reviewer called out and it's my daily driver. The pinch/wheel path is the one that worries me too — if you add debouncing or coalescing, I'll try to reproduce continuous pinch on a real trackpad and report the number of writes / any EPERM.

For reference, my current workaround is --force-device-scale-factor=1.25 in the shortcut, which is why I can't just "test the behavior" without an actual build — the whole point of the patch is per-window zoom, which the shortcut hack replaces.

Bumping with a fresh data point: Desktop 2.1.0 (the current release) still resets zoom to 100% after a restart. The reset is still present on dev — packages/desktop/src/main/windows/appearance.ts, wireZoom calls win.webContents.setZoomFactor(1) and there is no persisted value — so this fix did not make it into 2.1.0.

This PR is in a mergeable state: no conflicts, bot checks pass, template-compliant, small self-contained change (7 files). Requesting a code-owner review from @Hona and @Brendonovich so it can land before the next release.

Happy to adjust anything reviewers want; @wc4440222 has also offered Windows verification if a build is available.

Thanks for the review — all three points are addressed in d3bb151:

1. Debounced persistence. setZoomFactor now coalesces store writes (250 ms) and flushes on before-quit, so a continuous pinch or ctrl+wheel no longer triggers a burst of whole-file settings writes. Window creation still restores without persisting (wireZoom calls webContents.setZoomFactor directly), so the early/regular window paths don't write either.
2. Smaller test. Removed the Electron integration test and its helper (the src/main/windows/zoom/ bundle, the two-process spawn and the timeouts). Added zoom-factor.test.ts, a pure unit test of normalizeZoomFactor covering missing, wrong-type, out-of-range and valid values. It needs no Electron, so it actually runs in CI.
3. early.ts consistency. The early window now passes titlebarOverlay(tone(), storedZoomFactor()), matching windowAppearance, so the first Windows window's caption buttons size correctly before the renderer calls setTitlebar.

I also moved the 0.2–10 range and the fallback rule into zoom-factor.ts, shared by appearance.ts and defaults.ts, instead of duplicating the constants.

I still can't run bun on this machine, so CI will do the typecheck and the new unit test. @wc4440222 the branch is updated if a build becomes available for the Windows EPERM/pinch check you offered.

Reviewed the new revision (d3bb151). The debounce + unit-test direction is right, and I checked the titlebar part against the branch: updateTitlebar() already calls win.setTitleBarOverlay(overlay(theme, win.webContents.getZoomFactor())), so the overlay follows the current zoom at runtime, not just the restored one. That one is correct.

A few things I'd want addressed before this lands, ordered by how confident I am that they matter on Windows:

1. flushZoomFactor() can take the main process down (this is the EPERM case I offered to test)

It runs from a setTimeout callback and from before-quit. If getStore().set() throws — EPERM when something holds the settings file open (antivirus, OneDrive/Dropbox, backup agents do this routinely on Windows), or ENOSPC — that becomes an uncaught exception in the main process. From before-quit it can also abort the quit. Please wrap it:

try { ... } catch { /* keep the app alive, losing one zoom write is fine */ }

Dropping a zoom value is acceptable; crashing the app from a timer callback is not.

2. Is zoom meant to be global or per-window?

ZOOM_FACTOR_KEY is one app-wide key, but the surrounding code is clearly multi-window: WINDOW_IDS_KEY, windowArguments(id) in early.ts, and wireZoom(win) / updateTitlebar(win) are all per-window. With two windows at different zoom levels, the last change wins and both restore that value on restart — the window that did not change jumps. If app-wide is deliberate (like a browser profile setting), a one-line comment would settle it; otherwise key it by window id.

3. Discrete changes should not wait for the debounce

app.exit() does not emit before-quit, and a crash, kill, or Windows shutdown skips the flush too. Ctrl +/- and the View menu are discrete one-shot actions: persisting those immediately while debouncing only the continuous zoom-changed path removes the 250 ms loss window for the common case and still keeps pinch cheap.

4. The risk moved into the new code, so it needs coverage

Removing the Electron integration test was the right call, but the only test left covers normalizeZoomFactor. The new behaviour — coalescing, flush on quit, skipping redundant writes — now has zero coverage. If the coalescer were a small module with injected write/delay, a fake-timer unit test could assert: N rapid calls → 1 write, flush() → 1 write, unchanged value → 0 writes. All of that runs in CI without Electron.

5. Two smaller consistency notes

• windowAppearance sets webPreferences.zoomFactor and then wireZoom calls setZoomFactor with the same value. Harmless, but two sources of truth for the initial factor; keeping only the webPreferences one (which applies before first paint) makes the intent clearer.
• Restore uses normalizeZoomFactor (out of range → 1) while runtime uses clampZoom (out of range → nearest bound). A hand-edited zoomFactor: 12 restores to 100% instead of clamping to 1000%. Either policy is defensible, but they should probably agree.

On testing: I still can't build the desktop app on this machine, so I need an artifact (CI build or a packaged build) to run the Windows EPERM/pinch check — happy to test it against 2.1.2 as the baseline.

Thanks — this is a much better review than the automated one. Addressed in a5fe2d4:

1. flush can't crash the process. The store read/write now sits inside try/catch in the writer passed to the coalescer, so EPERM/ENOSPC (antivirus, OneDrive, backup agents) just drops that write instead of throwing from a timer callback or aborting the quit.

2. Global vs per-window. It is intentionally app-wide, and I added a comment saying so next to the key: this fixes "remember my zoom" (Desktop zoom resets to 100% after restarting the app #50168), which is a profile-level preference. Per-window zoom is a different feature and would be a follow-up; I didn't want to change the behaviour of the bug fix. Happy to key it by window id if maintainers prefer that as the target model.

3. Discrete changes are immediate. The coalescer (new zoom-persist.ts) is leading + trailing: the first change in a burst is written immediately, and only a continuous pinch/wheel is collapsed into at most one trailing write per 250 ms. before-quit flushes the pending value. app.exit() still skips before-quit, but that only loses a change made inside the 250 ms tail of a pinch — a discrete Ctrl +/- or menu action is already on disk.

4. The coalescer has CI coverage. It's a small module with injected write/now/schedule/cancel, and zoom-persist.test.ts asserts: rapid calls → one trailing write, flush() → one write, an unchanged value → no write. No Electron needed, so it runs in CI.

5. Consistency.

   • Removed the duplicate restore in wireZoom; webPreferences.zoomFactor (set in windowAppearance and in the early window) is now the single source, applied before first paint.
   • Restore and runtime share one normalizeZoomFactor: non-numeric → 1, numeric → clamped to 0.2–10. A hand-edited 12 now restores to 10, not 100%.

bun still isn't available on this machine, so CI will run the typecheck and both unit tests. @wc4440222 the branch is updated — if you can get a build (CI artifact or packaged), the Windows EPERM/pinch check you offered is exactly what remains unverified on real hardware.

Status update, in case it helps with review: the automated review is complete (No new concerns), every review thread is resolved, and the PR is mergeable with bun test (7 tests) and bun run typecheck passing in packages/desktop.

The remaining step is a code-owner review. @Hona @Brendonovich — whenever you have a moment. It's a small, self-contained change (7 files) for a regression that is still present on dev (2.1.4): wireZoom resets to 100% on every window creation and no zoom value is persisted. The fix stores it in the appearance settings and restores it, with coalesced writes and unit tests.

If this isn't the direction you want for the desktop, say so and I'll close it — otherwise it's ready to merge. No rush on my side; just flagging that the machine review is done so a human glance is the only thing left.

Thanks for working on this, and for the cross-platform testing in the thread. Zoom persistence landed in #59277: Settings → Appearance now has a Zoom select (100–200%), and the chosen level is saved and restored on launch. Keyboard and menu zoom step through the same levels. We went with fixed levels instead of saving any zoom factor, so pinch zoom stays temporary. Closing this in favour of that change.