From 256084270e6e219f69f03b8fae0c95ef1c8b7d42 Mon Sep 17 00:00:00 2001 From: Quickbeard Date: Fri, 14 Aug 2026 15:21:08 +0700 Subject: [PATCH] fix(desktop): move settings-store writes off the main thread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit conf v15's store.set is a synchronous fsync on the Electron main thread, and one of those parked the whole app in uninterruptible disk sleep when a kernel journal stall lost its wakeup (an NVMe write the drive never completed wedged ext4's journal; fast-commit then dropped the waiter): event loop dead, IPC dead, quit impossible. Renderers persist through store-set at exactly the busy moments, e.g. a sandbox mount-apply restart, which is where it was hit live — twice. getStore now returns a small fork-owned file store: state lives in memory (read-your-writes; conf's reread-per-access design would let a set on one key load a stale file and drop a still-queued update, which is why the stock class could not simply be rerouted), and every change flushes write-behind through a serialized latest-wins async atomic writer (temp file -> fsync -> rename via fs.promises, so a wedged disk stalls a threadpool thread while the app stays interactive). The on-disk format is unchanged and existing files load as-is; a corrupt file starts empty with a logged error instead of throwing from every accessor. stopSidecars awaits a bounded flushStoreWrites(1500) so relaunch/exit keep pending settings without letting a wedged disk hang shutdown; deleting a store file discards its writer first so a late flush cannot resurrect it. The electron-store dependency is removed. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 9 ++ DIVERGENCES.md | 1 + bun.lock | 27 ----- packages/desktop/package.json | 1 - packages/desktop/src/main/index.ts | 5 + packages/desktop/src/main/store-file.test.ts | 99 +++++++++++++++++++ packages/desktop/src/main/store-file.ts | 65 ++++++++++++ .../desktop/src/main/store-writer.test.ts | 96 ++++++++++++++++++ packages/desktop/src/main/store-writer.ts | 94 ++++++++++++++++++ packages/desktop/src/main/store.ts | 50 +++++++--- 10 files changed, 406 insertions(+), 41 deletions(-) create mode 100644 packages/desktop/src/main/store-file.test.ts create mode 100644 packages/desktop/src/main/store-file.ts create mode 100644 packages/desktop/src/main/store-writer.test.ts create mode 100644 packages/desktop/src/main/store-writer.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index eca3536214..aa1baab8ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,15 @@ file is the human-readable summary of what changed and why. ### Desktop / Web UI +- **A stalling disk can no longer freeze the app** — saving any setting + (window layout, server list, sandbox folders) used to write to disk + synchronously on the app's main thread. If the operating system's disk + journal stalled at that moment — observed on a machine whose NVMe drive + dropped a write during a power-state transition — the entire app froze + beyond even quitting, and had to be killed. Settings now save in the + background: the app stays responsive through a disk stall and the save + lands when the disk recovers. On quit and relaunch, pending saves get a + bounded window to finish so a wedged disk cannot hang shutdown either. - **Sandboxes can mount more folders after creation** — a sandbox used to be locked to the one project folder chosen when it was created. Each sandbox now has a **Folders…** entry in its menu (Settings → Servers) to mount diff --git a/DIVERGENCES.md b/DIVERGENCES.md index 2b8b8adf97..e3731211fc 100644 --- a/DIVERGENCES.md +++ b/DIVERGENCES.md @@ -195,6 +195,7 @@ from the squash message first, or use a plain merge. | `packages/desktop/src/renderer/index.tsx` | Calls `installConsoleFormatting()` (fork-only `./console-format`) before anything else logs. | `spyRendererConsole` receives Chromium's flattened console-message string, so any renderer `console.*` with an object argument reached the terminal and the exported debug logs as `[object Object]`; the wrapper serializes structured arguments (Error-aware, circular-safe, truncated) at the source. | Keep the import and call; take upstream's surrounding entry changes. | | `packages/desktop/src/main/updater.ts` | `autoUpdater.allowDowngrade` `true` → `false`. | Upstream's `true` is safe against immutable GitHub release tags. The fork's feed is a mutable bucket path, where re-uploading or failing to replace a stale `latest.yml` would actively downgrade every client on the next 10-minute poll. | Keep `false`; take upstream's other updater settings. `channel = "latest"` and `allowPrerelease = false` stay as upstream has them — the first must match `publish.channel` in `electron-builder.config.ts`, the second is inert for the `generic` provider (it only steers GitHub tag resolution). | | Dual-path WSL install: `packages/desktop/src/main/wsl/{runtime,servers}.ts` | `runtime.ts`: three internals gain `export` (`runPowerShell`, `runInteractiveCommand`, `resolveSystem32Command`) — no behavior change. `servers.ts`: the `install-wsl` job calls `installWslRuntimeDualPath` and the `install-distro` job calls `installWslDistroDualPath` (both from the fork-only `install-bundle.ts`, which holds all new logic) instead of `installWslRuntimeElevated` / `installWslDistro` directly; both return the same `WslCommandResult` shape, so the surrounding job/error handling is untouched. | The customer fleet sits behind whitelist-only proxies that cannot reach Microsoft's servers. The dual path downloads the pinned bundle (`wsl.zip`: runtime MSI + Ubuntu-24.04 image) from the codev storage host, enables VirtualMachinePlatform via offline dism + installs the MSI behind one UAC prompt, and falls back to upstream's `wsl --install` / `--web-download` paths only when the bundle cannot be *obtained* — a completed-but-failed elevated install is returned as-is, never retried behind a second UAC prompt. | Keep the two call swaps and the three `export` keywords; take upstream's surrounding changes. If upstream reworks `installWslRuntimeElevated` / `installWslDistro`, keep them exported from `runtime.ts` — `install-bundle.ts` calls them as the fallback path. | +| Async settings stores: `packages/desktop/src/main/{store,index}.ts`, `packages/desktop/package.json` | `store.ts` no longer uses electron-store: `getStore` now returns the fork-only `store-file.ts` store — in-memory state, flushed write-behind through `store-writer.ts`'s serialized latest-wins async atomic writer (temp → fsync → rename via `fs.promises`, so the fsync runs on the libuv threadpool). Adds `flushStoreWrites(timeoutMs)`; `removeStoreFile`/`removeStoreFileIfEmpty` discard/flush the writer around the delete so a late flush cannot resurrect the file. `index.ts`: `stopSidecars` awaits `flushStoreWrites(1500)`. `package.json`: the `electron-store` dependency is removed. On-disk format is unchanged (flat keys, tab-indented JSON, no extension) — existing files load as-is; a corrupt file now starts empty with a logged error instead of throwing from every accessor. | conf v15's `store.set` is a synchronous fsync on the Electron main thread, and one of those parked the whole app in uninterruptible disk sleep (kernel journal stall + lost ext4 fast-commit wakeup, observed live during a sandbox mount-apply restart): event loop dead, IPC dead, quit impossible. conf also rereads the file on every access, which makes write-behind unsound with the stock class (a set on one key reads a stale file and drops a still-queued update), so the storage layer had to be replaced rather than patched. | Keep the fork store; take upstream's surrounding changes. If upstream adds electron-store options (schema, migrations, dot-notation) or new store call sites, extend `store-file.ts` to match — its API is the superset the desktop actually uses (`get/set/has/delete/clear/store/path`). Do not reintroduce a synchronous write path on the main thread. | | Desktop sandbox wiring: `packages/desktop/src/main/{index,store-keys}.ts`, `packages/desktop/package.json` | Hooks the fork-only `src/main/sandbox/{bundle,controller,ipc}.ts` modules into startup: `index.ts` constructs the sandbox controller beside the WSL servers controller, adds `sandboxes.stopAll()` to `stopSidecars`, registers the `sandbox-*` IPC channels next to the WSL ones, and calls `sandboxes.initialize()` (all platforms) once the local server is ready; `store-keys.ts` gains `SANDBOXES_KEY = "sandboxes"`; `package.json` gains the `@codev/sandbox` and `@codev/gateway` workspace devDependencies (bundled into the main process by electron-vite, so not runtime dependencies; gateway supplies the sandbox egress broker's credentials and config fragment). | The desktop app is where sandboxes are created and run; `createSandboxController` generalizes the proven WSL-servers controller pattern to the per-platform `@codev/sandbox` providers (macOS vz / Windows wsl / Linux bwrap). | Keep the wiring lines and take upstream's surrounding changes; they are additive and order-insensitive apart from `initialize()` needing the local server ready first. Do not rename the `sandbox-*` channels or the `sandboxes` store key — preload/renderer and persisted sandbox configs depend on them. | | Desktop sandbox renderer slice: `packages/app/package.json`, `packages/app/src/{index.ts,app.tsx}`, `packages/app/src/context/{platform,server}.tsx`, `packages/app/src/components/{settings-v2/servers.tsx,dialog-select-server.tsx}`, `packages/app/src/wsl/settings.tsx`, `packages/app/src/i18n/{en,es}.ts`, `packages/desktop/src/preload/{index,types}.ts`, `packages/desktop/src/renderer/index.tsx` | Surfaces the fork-only sandbox modules (`packages/app/src/sandbox/*`, `packages/desktop/src/renderer/sandbox/*`) in the UI: app `package.json` exports `./sandbox/types` (mirror of `./wsl/types`); `index.ts` re-exports `useSandboxes` + the sandbox types; `app.tsx` mounts `SandboxesProvider` inside `WslServersProvider`; `platform.tsx` gains optional `sandboxes?: SandboxesPlatform`; `server.tsx` adds the third `ServerConnection.Sidecar` variant `{ variant: "sandbox"; name; projectDir?; mounts? }` with key `sandbox:` (`projectDir` is the host folder mounted into the guest and `mounts` the extra host folders with their resolved guest paths — the guest only ever sees its own mount points, so this is the only way the app can name a sandbox's projects after the folders it works on); `settings-v2/servers.tsx` filters sandbox rows out of the generic list and renders `SandboxServerSettings` beside the WSL section; `dialog-select-server.tsx` `handleRemove` routes `sandbox:` keys to `platform.sandboxes.remove` (like `wsl:` keys) and moves the `tabs.removeServer(key)` + `server.remove(key)` pair *above* both awaited platform removals; `wsl/settings.tsx` `AddServerMenu` shows the menu when either wslServers or sandboxes exist and gains an "Add sandbox" item; i18n gains the `sandbox.*` key family (including the optional per-sandbox CPU/memory fields, macOS-only since only the vz backend applies them; `sandbox.add.projectDirHint`, the standing explanation above the folder field; and the `sandbox.mounts.*` family for the manage-folders dialog that adds/removes extra mounts on an existing sandbox); preload exposes `api.sandboxes` over the `sandbox-*` channels and `renderer/index.tsx` wires the platform api, loading gate, ready-connection list, and the `startupServer` fallback (fork-only wrapper that validates `sandbox:` default keys the way `availableStartupServer` validates `wsl:` ones). | Ready sandboxes must appear as first-class servers and be manageable from settings; the slice mirrors the WSL vertical exactly so upstream conflicts resolve the same way in both. Removing a server destroys a disk and takes seconds, and the connection leaves the list the moment it finishes, so the tab cleanup has to happen before the await rather than after it. | Keep every listed edit; take upstream's surrounding changes. If upstream adds a `Sidecar` variant or reworks `AddServerMenu`, re-apply the sandbox variant/menu item on top. The `sandbox:` key format is persisted as the default-server key and as `config.id` in the main-process store — never change one side alone. Keep the tab cleanup ahead of the platform removals in `handleRemove`; if upstream reorders it back, re-apply. | diff --git a/bun.lock b/bun.lock index 4a20d4572f..4b9cb1889b 100644 --- a/bun.lock +++ b/bun.lock @@ -410,7 +410,6 @@ "effect": "catalog:", "electron-context-menu": "4.1.2", "electron-log": "^5", - "electron-store": "11.0.2", "electron-updater": "6.8.9", "electron-window-state": "^5.0.3", "marked": "^15", @@ -3027,8 +3026,6 @@ "atomic-sleep": ["atomic-sleep@1.0.0", "", {}, "sha512-kNOjDqAh7px0XWNI+4QbzoiR/nTkHAWNud2uvnJquD1/x5a7EQZMJT0AczqK0Qn67oY/TTQ1LbUKajZpp3I9tQ=="], - "atomically": ["atomically@2.1.1", "", { "dependencies": { "stubborn-fs": "^2.0.0", "when-exit": "^2.1.4" } }, "sha512-P4w9o2dqARji6P7MHprklbfiArZAWvo07yW7qs3pdljb3BWr12FIB7W+p0zJiuiVsUpRO0iZn1kFFcpPegg0tQ=="], - "autoprefixer": ["autoprefixer@10.5.0", "", { "dependencies": { "browserslist": "^4.28.2", "caniuse-lite": "^1.0.30001787", "fraction.js": "^5.3.4", "picocolors": "^1.1.1", "postcss-value-parser": "^4.2.0" }, "peerDependencies": { "postcss": "^8.1.0" }, "bin": { "autoprefixer": "bin/autoprefixer" } }, "sha512-FMhOoZV4+qR6aTUALKX2rEqGG+oyATvwBt9IIzVR5rMa2HRWPkxf+P+PAJLD1I/H5/II+HuZcBJYEFBpq39ong=="], "available-typed-arrays": ["available-typed-arrays@1.0.7", "", { "dependencies": { "possible-typed-array-names": "^1.0.0" } }, "sha512-wvUjBtSGN7+7SjNpq/9M2Tg350UZD3q62IFZLbRAR1bSMlCo1ZaeW+BJ+D090e4hIIZLBcTDWe4Mh4jvUDajzQ=="], @@ -3229,8 +3226,6 @@ "condense-newlines": ["condense-newlines@0.2.1", "", { "dependencies": { "extend-shallow": "^2.0.1", "is-whitespace": "^0.3.0", "kind-of": "^3.0.2" } }, "sha512-P7X+QL9Hb9B/c8HI5BFFKmjgBu2XpQuF98WZ9XkO+dBGgk5XgwiQz7o1SmpglNWId3581UcS0SFAWfoIhMHPfg=="], - "conf": ["conf@15.1.0", "", { "dependencies": { "ajv": "^8.17.1", "ajv-formats": "^3.0.1", "atomically": "^2.0.3", "debounce-fn": "^6.0.0", "dot-prop": "^10.0.0", "env-paths": "^3.0.0", "json-schema-typed": "^8.0.1", "semver": "^7.7.2", "uint8array-extras": "^1.5.0" } }, "sha512-Uy5YN9KEu0WWDaZAVJ5FAmZoaJt9rdK6kH+utItPyGsCqCgaTKkrmZx3zoE0/3q6S3bcp3Ihkk+ZqPxWxFK5og=="], - "confbox": ["confbox@0.2.4", "", {}, "sha512-ysOGlgTFbN2/Y6Cg3Iye8YKulHw+R2fNXHrgSmXISQdMnomY6eNDprVdW9R5xBguEqI954+S6709UyiO7B+6OQ=="], "config-chain": ["config-chain@1.1.13", "", { "dependencies": { "ini": "^1.3.4", "proto-list": "~1.2.1" } }, "sha512-qj+f8APARXHrM0hraqXYb2/bOVSV4PvJQlNZ/DVj0QrmNM2q2euizkeuVckQ57J+W0mRH6Hvi+k50M4Jul2VRQ=="], @@ -3303,8 +3298,6 @@ "db0": ["db0@0.3.4", "", { "peerDependencies": { "@electric-sql/pglite": "*", "@libsql/client": "*", "better-sqlite3": "*", "drizzle-orm": "*", "mysql2": "*", "sqlite3": "*" }, "optionalPeers": ["@electric-sql/pglite", "@libsql/client", "better-sqlite3", "drizzle-orm", "mysql2", "sqlite3"] }, "sha512-RiXXi4WaNzPTHEOu8UPQKMooIbqOEyqA1t7Z6MsdxSCeb8iUC9ko3LcmsLmeUt2SM5bctfArZKkRQggKZz7JNw=="], - "debounce-fn": ["debounce-fn@6.0.0", "", { "dependencies": { "mimic-function": "^5.0.0" } }, "sha512-rBMW+F2TXryBwB54Q0d8drNEI+TfoS9JpNTAoVpukbWEhjXQq4rySFYLaqXMFXwdv61Zb2OHtj5bviSoimqxRQ=="], - "debug": ["debug@4.4.3", "", { "dependencies": { "ms": "^2.1.3" }, "peerDependencies": { "supports-color": "*" }, "optionalPeers": ["supports-color"] }, "sha512-RGwwWnwQvkVfavKVt22FGLw+xYSdzARwm0ru6DhTVA3umU5hZc28V3kO4stgYryrTlLpuvgI9GiijltAjNbcqA=="], "decimal.js": ["decimal.js@10.5.0", "", {}, "sha512-8vDa8Qxvr/+d94hSh5P3IJwI5t8/c0KsMp+g8bNw9cY2icONa5aPfvKeieW1WlG0WQYwwhJ7mjui2xtiePQSXw=="], @@ -3425,8 +3418,6 @@ "electron-publish": ["electron-publish@26.15.1", "", { "dependencies": { "@types/fs-extra": "^9.0.11", "aws4": "^1.13.2", "builder-util": "26.15.0", "builder-util-runtime": "9.7.0", "chalk": "^4.1.2", "form-data": "^4.0.5", "fs-extra": "^10.1.0", "lazy-val": "^1.0.5", "mime": "^2.5.2" } }, "sha512-BMgMHOyexWn0UnOC+Afffw0DMrr0yfLp4U8YsLXwoJ3Da7LS7WUnz21teYZqO0gaApE1KgsjREWmbPqvF5JcPg=="], - "electron-store": ["electron-store@11.0.2", "", { "dependencies": { "conf": "^15.0.2", "type-fest": "^5.0.1" } }, "sha512-4VkNRdN+BImL2KcCi41WvAYbh6zLX5AUTi4so68yPqiItjbgTjqpEnGAqasgnG+lB6GuAyUltKwVopp6Uv+gwQ=="], - "electron-to-chromium": ["electron-to-chromium@1.5.364", "", {}, "sha512-G/dYE3+AYhyHwzTwg8UbnXf7zqMERYh7l2jJ3QujhFsH8agSYwtnGAR2aZ7f0AakIKJXd5En/Hre4igIUrdlYw=="], "electron-updater": ["electron-updater@6.8.9", "", { "dependencies": { "builder-util-runtime": "9.7.0", "fs-extra": "^10.1.0", "js-yaml": "^4.1.0", "lazy-val": "^1.0.5", "lodash.escaperegexp": "^4.1.2", "lodash.isequal": "^4.5.0", "semver": "~7.7.3", "tiny-typed-emitter": "^2.1.0" } }, "sha512-ZhVxM9iGONUpZGI1FxdMRgJjUFXi7AYGVa5PwKlO1tV1/4zDxQmfKpXOHVztKrd6L9rLcFjERvi1Mf2vxyTkig=="], @@ -4101,8 +4092,6 @@ "mimic-fn": ["mimic-fn@4.0.0", "", {}, "sha512-vqiC06CuhBTUdZH+RYl8sFrL096vA45Ok5ISO6sE/Mr1jRbGH4Csnhi8f3wKVl7x8mO4Au7Ir9D3Oyv1VYMFJw=="], - "mimic-function": ["mimic-function@5.0.1", "", {}, "sha512-VP79XUPxV2CigYP3jWwAUFSku2aKqBH7uTAapFWCBqutsbmDo96KY5o8uh6U+/YSIn5OxJnXp73beVkpqMIGhA=="], - "mimic-response": ["mimic-response@3.1.0", "", {}, "sha512-z0yWI+4FDrrweS8Zmt4Ej5HdJmky15+L2e6Wgn3+iK5fWzb6T3fhNFq2+MeTRb064c6Wr4N/wv0DzQTjNzHNGQ=="], "min-indent": ["min-indent@1.0.1", "", {}, "sha512-I9jwMn07Sy/IwOj3zVkVik2JTvgpaykDZEigL6Rx6N9LbMywwUSMtxET+7lVoDLLd3O3IXwJwvuuns8UB/HeAg=="], @@ -4789,10 +4778,6 @@ "strnum": ["strnum@1.1.2", "", {}, "sha512-vrN+B7DBIoTTZjnPNewwhx6cBA/H+IS7rfW68n7XxC1y7uoiGQBxaKzqucGUgavX15dJgiGztLJ8vxuEzwqBdA=="], - "stubborn-fs": ["stubborn-fs@2.0.0", "", { "dependencies": { "stubborn-utils": "^1.0.1" } }, "sha512-Y0AvSwDw8y+nlSNFXMm2g6L51rBGdAQT20J3YSOqxC53Lo3bjWRtr2BKcfYoAf352WYpsZSTURrA0tqhfgudPA=="], - - "stubborn-utils": ["stubborn-utils@1.0.2", "", {}, "sha512-zOh9jPYI+xrNOyisSelgym4tolKTJCQd5GBhK0+0xJvcYDcwlOoxF/rnFKQ2KRZknXSG9jWAp66fwP6AxN9STg=="], - "style-to-object": ["style-to-object@1.0.14", "", { "dependencies": { "inline-style-parser": "0.2.7" } }, "sha512-LIN7rULI0jBscWQYaSswptyderlarFkjQ+t79nzty8tcIAceVomEVlLzH5VP4Cmsv6MtKhs7qaAiwlcp+Mgaxw=="], "sucrase": ["sucrase@3.35.1", "", { "dependencies": { "@jridgewell/gen-mapping": "^0.3.2", "commander": "^4.0.0", "lines-and-columns": "^1.1.6", "mz": "^2.7.0", "pirates": "^4.0.1", "tinyglobby": "^0.2.11", "ts-interface-checker": "^0.1.9" }, "bin": { "sucrase": "bin/sucrase", "sucrase-node": "bin/sucrase-node" } }, "sha512-DhuTmvZWux4H1UOnWMB3sk0sbaCVOoQZjv8u1rDoTV0HTdGem9hkAZtl4JZy8P2z4Bg0nT+YMeOFyVr4zcG5Tw=="], @@ -4809,8 +4794,6 @@ "system-architecture": ["system-architecture@0.1.0", "", {}, "sha512-ulAk51I9UVUyJgxlv9M6lFot2WP3e7t8Kz9+IS6D4rVba1tR9kON+Ey69f+1R4Q8cd45Lod6a4IcJIxnzGc/zA=="], - "tagged-tag": ["tagged-tag@1.0.0", "", {}, "sha512-yEFYrVhod+hdNyx7g5Bnkkb0G6si8HJurOoOEgC8B/O0uXLHlaey/65KRv6cuWBNhBgHKAROVpc7QyYqE5gFng=="], - "tailwindcss": ["tailwindcss@4.1.11", "", {}, "sha512-2E9TBm6MDD/xKYe+dvJZAmg3yxIEDNRc0jwlNyDg/4Fil2QcSLjFKGVff0lAf1jjeaArlG/M75Ey/EYr/OJtBA=="], "tapable": ["tapable@2.3.3", "", {}, "sha512-uxc/zpqFg6x7C8vOE7lh6Lbda8eEL9zmVm/PLeTPBRhh1xCgdWaQ+J1CUieGpIfm2HdtsUpRv+HshiasBMcc6A=="], @@ -4921,8 +4904,6 @@ "typescript": ["typescript@5.8.2", "", { "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" } }, "sha512-aJn6wq13/afZp/jT9QZmwEjDqqvSGp1VT5GVg+f/t6/oVyrgXM6BY1h9BRh/O5p3PlUPAe+WuiEZOmb/49RqoQ=="], - "uint8array-extras": ["uint8array-extras@1.5.0", "", {}, "sha512-rvKSBiC5zqCCiDZ9kAOszZcDvdAHwwIKJG33Ykj43OKcWsnmcBRL09YTU4nOeHZ8Y2a7l1MgTd08SBe9A8Qj6A=="], - "ulid": ["ulid@3.0.1", "", { "bin": { "ulid": "dist/cli.js" } }, "sha512-dPJyqPzx8preQhqq24bBG1YNkvigm87K8kVEHCD+ruZg24t6IFEFv00xMWfxcC4djmFtiTLdFuADn4+DOz6R7Q=="], "unbox-primitive": ["unbox-primitive@1.1.0", "", { "dependencies": { "call-bound": "^1.0.3", "has-bigints": "^1.0.2", "has-symbols": "^1.1.0", "which-boxed-primitive": "^1.1.1" } }, "sha512-nWJ91DjeOkej/TA8pXQ3myruKpKEYgqvpw9lz4OPHj/NWFNluYrjbz9j01CJ8yKQd2g4jFoOkINCTW2I5LEEyw=="], @@ -5029,8 +5010,6 @@ "whatwg-url": ["whatwg-url@5.0.0", "", { "dependencies": { "tr46": "~0.0.3", "webidl-conversions": "^3.0.0" } }, "sha512-saE57nupxk6v3HY35+jzBwYa0rKSy0XR8JSxZPwgLr7ys0IBzhGviA1/TUGJLmSVqs8pb9AnvICXEuOHLprYTw=="], - "when-exit": ["when-exit@2.1.5", "", {}, "sha512-VGkKJ564kzt6Ms1dbgPP/yuIoQCrsFAnRbptpC5wOEsDaNsbCB2bnfnaA8i/vRs5tjUSEOtIuvl9/MyVsvQZCg=="], - "which": ["which@6.0.1", "", { "dependencies": { "isexe": "^4.0.0" }, "bin": { "node-which": "bin/which.js" } }, "sha512-oGLe46MIrCRqX7ytPUf66EAYvdeMIZYn3WaocqqKZAxrBpkqHfL/qvTyJ/bTk5+AqHCjXmrv3CEWgy368zhRUg=="], "which-boxed-primitive": ["which-boxed-primitive@1.1.1", "", { "dependencies": { "is-bigint": "^1.1.0", "is-boolean-object": "^1.2.1", "is-number-object": "^1.1.1", "is-string": "^1.1.1", "is-symbol": "^1.1.1" } }, "sha512-TbX3mj8n0odCBFVlY8AxkqcHASw3L60jIuF8jFP78az3C2YhmGvqbHBpAjTRH2/xqYunrJ9g1jSyjCjpoWzIAA=="], @@ -5665,8 +5644,6 @@ "condense-newlines/kind-of": ["kind-of@3.2.2", "", { "dependencies": { "is-buffer": "^1.1.5" } }, "sha512-NOW9QQXMoZGg/oqnVNoNTTIFEIid1627WCffUBJEdMxYApq7mNE7CpzucIPc+ZQg25Phej7IJSmX3hO+oblOtQ=="], - "conf/dot-prop": ["dot-prop@10.1.0", "", { "dependencies": { "type-fest": "^5.0.0" } }, "sha512-MVUtAugQMOff5RnBy2d9N31iG0lNwg1qAoAOn7pOK5wf94WIaE3My2p3uwTQuvS2AcqchkcR3bHByjaM0mmi7Q=="], - "config-chain/ini": ["ini@1.3.8", "", {}, "sha512-JV/yugV2uzW5iMRSiZAyDtQd+nxtUnjeLt0acNdw98kKLrvuRVyB80tsREOE7yvGVgalhZ6RNXCmEHkUKBKxew=="], "crc/buffer": ["buffer@5.7.1", "", { "dependencies": { "base64-js": "^1.3.1", "ieee754": "^1.1.13" } }, "sha512-EHcyIPBQ4BSGlvjB16k5KgAJ27CIsHY/2JBmCRReo48y9rQ3MaUzWX3KVlBa4U7MyX02HdVj0K7C3WaB3ju7FQ=="], @@ -5703,8 +5680,6 @@ "electron-publish/mime": ["mime@2.6.0", "", { "bin": { "mime": "cli.js" } }, "sha512-USPkMeET31rOMiarsBNIHZKLGgvKc/LrjofAnBlOttf5ajRvqiRA8QsenbcooctK6d6Ts6aqZXBA+XbkKthiQg=="], - "electron-store/type-fest": ["type-fest@5.7.0", "", { "dependencies": { "tagged-tag": "^1.0.0" } }, "sha512-1URUxUqfHFM1c+zfSPsa3gnkO7Aq21qyH75SIduNYz4SzY964rn1X2vCMQaHSHhktiw+0kPa2iyb6PUpXqB6Vg=="], - "electron-updater/js-yaml": ["js-yaml@4.1.1", "", { "dependencies": { "argparse": "^2.0.1" }, "bin": { "js-yaml": "bin/js-yaml.js" } }, "sha512-qQKT4zQxXl8lLwBtHMWwaTcGfFOZviOJet3Oy/xmGk2gZH677CJM9EvtfdSkgWcATZhj/55JZ0rmy3myCT5lsA=="], "electron-updater/semver": ["semver@7.7.3", "", { "bin": { "semver": "bin/semver.js" } }, "sha512-SdsKMrI9TdgjdweUSR9MweHA4EJ8YxHn8DFaDisvhVlUOe4BF1tLD7GAj0lIqWVl+dPb/rExr0Btby5loQm20Q=="], @@ -6375,8 +6350,6 @@ "c12/chokidar/readdirp": ["readdirp@5.0.0", "", {}, "sha512-9u/XQ1pvrQtYyMpZe7DXKv2p5CNvyVwzUB6uhLAnQwHMSgKMBR62lc7AHljaeteeHXn11XTAaLLUVZYVZyuRBQ=="], - "conf/dot-prop/type-fest": ["type-fest@5.7.0", "", { "dependencies": { "tagged-tag": "^1.0.0" } }, "sha512-1URUxUqfHFM1c+zfSPsa3gnkO7Aq21qyH75SIduNYz4SzY964rn1X2vCMQaHSHhktiw+0kPa2iyb6PUpXqB6Vg=="], - "cross-spawn/which/isexe": ["isexe@2.0.0", "", {}, "sha512-RHxMLp9lnKHGHRng9QFhRCMbYAcVpn69smSGcq3f36xjgVVWThj4qqLbTLlq7Ssj8B+fIQ1EuCEGI2lKsyQeIw=="], "dir-compare/minimatch/brace-expansion": ["brace-expansion@1.1.15", "", { "dependencies": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" } }, "sha512-EwOCDEex4quD37XhqM3omwtMoJjr//isUZz1JopUNWms+4Z2ViyM/k1YIRePpoVNnQhENnxtFjLaxNHrT7xIUg=="], diff --git a/packages/desktop/package.json b/packages/desktop/package.json index 7eea2b724c..93e72c743b 100644 --- a/packages/desktop/package.json +++ b/packages/desktop/package.json @@ -29,7 +29,6 @@ "effect": "catalog:", "electron-context-menu": "4.1.2", "electron-log": "^5", - "electron-store": "11.0.2", "electron-updater": "6.8.9", "electron-window-state": "^5.0.3", "marked": "^15" diff --git a/packages/desktop/src/main/index.ts b/packages/desktop/src/main/index.ts index 7985b9d664..be8868daf4 100644 --- a/packages/desktop/src/main/index.ts +++ b/packages/desktop/src/main/index.ts @@ -48,6 +48,7 @@ import { registerWslIpcHandlers } from "./wsl/ipc" import { spawnWslSidecar } from "./wsl/sidecar" import { createDesktopSandboxController } from "./sandbox/controller" import { registerSandboxIpcHandlers } from "./sandbox/ipc" +import { flushStoreWrites } from "./store" import { migrate } from "./migrate" import { cleanupStoreFiles } from "./store-cleanup" @@ -172,6 +173,10 @@ const main = Effect.gen(function* () { await killSidecar() wslServers.stopAll() sandboxes.stopAll() + // Settings writes are asynchronous (see store.ts); relaunch, SIGINT and + // SIGTERM await this chain, so pending settings get a bounded window to + // land before the process exits. + await flushStoreWrites(1500) } const relaunch = () => { setAppQuitting() diff --git a/packages/desktop/src/main/store-file.test.ts b/packages/desktop/src/main/store-file.test.ts new file mode 100644 index 0000000000..42cfa8ce5c --- /dev/null +++ b/packages/desktop/src/main/store-file.test.ts @@ -0,0 +1,99 @@ +import { expect, test } from "bun:test" +import { mkdtemp, rm, writeFile } from "node:fs/promises" +import { tmpdir } from "node:os" +import { join } from "node:path" +import { createFileStore } from "./store-file" + +async function scratch() { + return mkdtemp(join(tmpdir(), "store-file-")) +} + +test("reads its own writes while the flush is still pending", async () => { + const dir = await scratch() + const store = createFileStore(join(dir, "settings")) + store.set("sandboxes", { sandboxes: [{ id: "sandbox:x" }] }) + expect(store.get("sandboxes")).toEqual({ sandboxes: [{ id: "sandbox:x" }] }) + await store.writer.flush() + expect(JSON.parse(await Bun.file(store.path).text())).toEqual({ sandboxes: { sandboxes: [{ id: "sandbox:x" }] } }) + await rm(dir, { recursive: true, force: true }) +}) + +test("a set on one key never drops a still-queued write to another", async () => { + // The failure mode that ruled out keeping electron-store: conf rereads the + // file per access, so with write-behind flushing, set("b") would load a + // stale file missing the queued set("a") and clobber it. + const dir = await scratch() + const store = createFileStore(join(dir, "settings")) + store.set("a", 1) + store.set("b", 2) + store.set("c", 3) + await store.writer.flush() + expect(JSON.parse(await Bun.file(store.path).text())).toEqual({ a: 1, b: 2, c: 3 }) + await rm(dir, { recursive: true, force: true }) +}) + +test("loads an existing electron-store-format file as-is", async () => { + const dir = await scratch() + const path = join(dir, "opencode.settings") + // electron-store with fileExtension "" wrote flat keys as tab-indented JSON. + await writeFile(path, `{\n\t"defaultServerUrl": "sandbox:codev-work",\n\t"sandboxes": {\n\t\t"sandboxes": []\n\t}\n}`) + const store = createFileStore(path) + expect(store.get("defaultServerUrl")).toBe("sandbox:codev-work") + expect(Object.keys(store.store)).toEqual(["defaultServerUrl", "sandboxes"]) + await rm(dir, { recursive: true, force: true }) +}) + +test("writes tab-indented JSON matching the previous on-disk format", async () => { + const dir = await scratch() + const store = createFileStore(join(dir, "settings")) + store.set("k", { nested: true }) + await store.writer.flush() + expect(await Bun.file(store.path).text()).toBe(`{\n\t"k": {\n\t\t"nested": true\n\t}\n}`) + await rm(dir, { recursive: true, force: true }) +}) + +test("delete and clear persist, and clear leaves an empty object file", async () => { + const dir = await scratch() + const store = createFileStore(join(dir, "settings")) + store.set("a", 1) + store.set("b", 2) + store.delete("a") + await store.writer.flush() + expect(JSON.parse(await Bun.file(store.path).text())).toEqual({ b: 2 }) + store.clear() + await store.writer.flush() + // store-cleanup's isEmptyStore treats {} as deletable. + expect(await Bun.file(store.path).text()).toBe("{}") + expect(store.store).toEqual({}) + await rm(dir, { recursive: true, force: true }) +}) + +test("a corrupt file is reported and starts empty instead of throwing", async () => { + const dir = await scratch() + const path = join(dir, "settings") + await writeFile(path, "{ not json") + const seen: unknown[] = [] + const store = createFileStore(path, { onError: (error) => seen.push(error) }) + expect(seen).toHaveLength(1) + expect(store.store).toEqual({}) + store.set("recovered", true) + await store.writer.flush() + expect(JSON.parse(await Bun.file(path).text())).toEqual({ recovered: true }) + await rm(dir, { recursive: true, force: true }) +}) + +test("a missing file is the normal first run: empty, no error", async () => { + const dir = await scratch() + const seen: unknown[] = [] + const store = createFileStore(join(dir, "settings"), { onError: (error) => seen.push(error) }) + expect(seen).toEqual([]) + expect(store.get("anything")).toBeUndefined() + await rm(dir, { recursive: true, force: true }) +}) + +test("setting undefined throws like electron-store did", async () => { + const dir = await scratch() + const store = createFileStore(join(dir, "settings")) + expect(() => store.set("k", undefined)).toThrow(TypeError) + await rm(dir, { recursive: true, force: true }) +}) diff --git a/packages/desktop/src/main/store-file.ts b/packages/desktop/src/main/store-file.ts new file mode 100644 index 0000000000..212d765967 --- /dev/null +++ b/packages/desktop/src/main/store-file.ts @@ -0,0 +1,65 @@ +import { readFileSync } from "node:fs" +import { createAsyncAtomicWriter } from "./store-writer" + +// In-memory JSON store flushed through the async atomic writer, replacing +// electron-store. conf v15 rereads the file on every access and fsyncs on +// every set from the main thread: the sync fsync parked the app in +// uninterruptible disk sleep when a kernel journal stall lost its wakeup, +// and the reread makes queued asynchronous writes unsound — a set on one key +// would load a stale file and drop a still-queued update to another. Keeping +// the state in memory gives read-your-writes for free and makes the disk a +// write-behind copy. The on-disk format is byte-compatible with what +// electron-store wrote for these options (flat keys, tab-indented JSON, no +// extension): existing files load as-is. +export type FileStore = ReturnType + +export function createFileStore(path: string, options?: { onError?: (error: unknown) => void }) { + const writer = createAsyncAtomicWriter(path, { onError: options?.onError }) + const state = loadState(path, options?.onError) + const persist = () => writer.write(JSON.stringify(state, undefined, "\t")) + return { + path, + writer, + get(key: string): unknown { + return state[key] + }, + has(key: string) { + return key in state + }, + set(key: string, value: unknown) { + // Matches electron-store: undefined is a bug at the call site, not a + // delete. + if (value === undefined) throw new TypeError(`Use delete() to clear "${key}"`) + state[key] = value + persist() + }, + delete(key: string) { + delete state[key] + persist() + }, + clear() { + for (const key of Object.keys(state)) delete state[key] + persist() + }, + get store(): Record { + return { ...state } + }, + } +} + +function loadState(path: string, onError?: (error: unknown) => void): Record { + try { + const parsed: unknown = JSON.parse(readFileSync(path, "utf8")) + if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) { + // Null prototype so stored keys like "constructor" stay plain data. + return Object.assign(Object.create(null) as Record, parsed) + } + return Object.create(null) as Record + } catch (error) { + // A missing file is the normal first run. A corrupt one is reported and + // starts empty — it regenerates on the next write, instead of throwing + // from every accessor the way conf without clearInvalidConfig does. + if ((error as NodeJS.ErrnoException).code !== "ENOENT") onError?.(error) + return Object.create(null) as Record + } +} diff --git a/packages/desktop/src/main/store-writer.test.ts b/packages/desktop/src/main/store-writer.test.ts new file mode 100644 index 0000000000..7173bab31a --- /dev/null +++ b/packages/desktop/src/main/store-writer.test.ts @@ -0,0 +1,96 @@ +import { expect, test } from "bun:test" +import { open, readdir, rename, unlink } from "node:fs/promises" +import { mkdtemp, rm } from "node:fs/promises" +import { tmpdir } from "node:os" +import { join } from "node:path" +import { createAsyncAtomicWriter } from "./store-writer" + +test("writes land atomically and rapid writes coalesce to the newest payload", async () => { + const dir = await mkdtemp(join(tmpdir(), "store-writer-")) + const target = join(dir, "settings") + const writer = createAsyncAtomicWriter(target) + writer.write("one") + writer.write("two") + writer.write("three") + expect(await writer.flush()).toBe(true) + expect(await Bun.file(target).text()).toBe("three") + // No temp litter after a clean flush. + expect((await readdir(dir)).filter((name) => name.includes(".tmp"))).toEqual([]) + await rm(dir, { recursive: true, force: true }) +}) + +test("sequential write/flush cycles each persist", async () => { + const dir = await mkdtemp(join(tmpdir(), "store-writer-")) + const target = join(dir, "settings") + const writer = createAsyncAtomicWriter(target) + writer.write("first") + await writer.flush() + expect(await Bun.file(target).text()).toBe("first") + writer.write("second") + await writer.flush() + expect(await Bun.file(target).text()).toBe("second") + await rm(dir, { recursive: true, force: true }) +}) + +test("flush times out instead of hanging when the disk wedges", async () => { + const writer = createAsyncAtomicWriter("/nowhere/settings", { + fsImpl: { + // A wedged journal looks like an fs call that never returns. + open: () => new Promise(() => undefined), + rename, + unlink, + }, + }) + writer.write("data") + expect(await writer.flush(50)).toBe(false) +}) + +test("write errors are reported and leave no temp file behind", async () => { + const dir = await mkdtemp(join(tmpdir(), "store-writer-")) + const target = join(dir, "settings") + const seen: unknown[] = [] + const writer = createAsyncAtomicWriter(target, { + onError: (error) => seen.push(error), + fsImpl: { + open, + rename: async () => { + throw new Error("rename failed") + }, + unlink, + }, + }) + writer.write("data") + expect(await writer.flush()).toBe(true) + expect(seen).toHaveLength(1) + expect(await Bun.file(target).exists()).toBe(false) + expect((await readdir(dir)).filter((name) => name.includes(".tmp"))).toEqual([]) + await rm(dir, { recursive: true, force: true }) +}) + +test("discard drops an in-flight payload so a deleted store file stays deleted", async () => { + const dir = await mkdtemp(join(tmpdir(), "store-writer-")) + const target = join(dir, "settings") + let releaseOpen: (() => void) | undefined + const gate = new Promise((resolve) => { + releaseOpen = resolve + }) + const writer = createAsyncAtomicWriter(target, { + fsImpl: { + open: async (path, flags) => { + await gate + return open(path, flags) + }, + rename, + unlink, + }, + }) + writer.write("resurrected?") + // The payload is already in flight (consumed from the queue); discard must + // still prevent the final rename. + writer.discard() + releaseOpen?.() + expect(await writer.flush()).toBe(true) + expect(await Bun.file(target).exists()).toBe(false) + expect((await readdir(dir)).filter((name) => name.includes(".tmp"))).toEqual([]) + await rm(dir, { recursive: true, force: true }) +}) diff --git a/packages/desktop/src/main/store-writer.ts b/packages/desktop/src/main/store-writer.ts new file mode 100644 index 0000000000..d40b208401 --- /dev/null +++ b/packages/desktop/src/main/store-writer.ts @@ -0,0 +1,94 @@ +import { open, rename, unlink } from "node:fs/promises" + +// Serialized, latest-wins, asynchronous atomic file writer for the settings +// stores. Exists because a synchronous fsync on the Electron main thread can +// park the whole app in uninterruptible disk sleep when the kernel's journal +// stalls — observed live: an NVMe write the drive never completed wedged the +// ext4 journal, fast-commit lost the wakeup, and the event loop never ran +// again. The same temp-write → fsync → rename dance done through fs.promises +// runs on the libuv threadpool, so a wedged disk stalls one pool thread while +// the app stays interactive and the flush lands whenever the disk recovers. +type WriterFs = { + open: typeof open + rename: typeof rename + unlink: typeof unlink +} + +export type AsyncAtomicWriter = ReturnType + +export function createAsyncAtomicWriter( + path: string, + options?: { onError?: (error: unknown) => void; fsImpl?: WriterFs }, +) { + const fsImpl = options?.fsImpl ?? { open, rename, unlink } + let pending: string | undefined + let flushing: Promise | undefined + let dropped = false + let sequence = 0 + + const flushLoop = async () => { + while (pending !== undefined) { + const data = pending + pending = undefined + // Unique temp names: a slow flush must never clobber a newer one's + // temp file mid-write. + const temp = `${path}.${process.pid}.${sequence++}.tmp` + try { + const handle = await fsImpl.open(temp, "w") + try { + await handle.writeFile(data) + await handle.sync() + } finally { + await handle.close() + } + // A discard while this payload was in flight means the store file is + // being deleted; renaming now would resurrect it. + if (dropped) { + await fsImpl.unlink(temp).catch(() => undefined) + continue + } + await fsImpl.rename(temp, path) + } catch (error) { + options?.onError?.(error) + await fsImpl.unlink(temp).catch(() => undefined) + } + } + flushing = undefined + } + + return { + // Never blocks: queues the payload and returns. Writes queued while a + // flush is in flight coalesce to the newest payload. + write(data: string) { + pending = data + dropped = false + flushing ??= flushLoop() + }, + // Drops queued and in-flight payloads without writing them, so deleting + // the store file cannot race a late flush that brings it back. + discard() { + pending = undefined + dropped = true + }, + // Resolves true when everything queued so far is on disk (or failed and + // was reported), false when the timeout elapsed first — shutdown must + // not hang on a wedged disk. + async flush(timeoutMs?: number) { + const current = flushing + if (!current) return true + if (timeoutMs === undefined) { + await current + return true + } + let timer: ReturnType | undefined + const outcome = await Promise.race([ + current.then(() => true), + new Promise((resolve) => { + timer = setTimeout(() => resolve(false), timeoutMs) + }), + ]) + clearTimeout(timer) + return outcome + }, + } +} diff --git a/packages/desktop/src/main/store.ts b/packages/desktop/src/main/store.ts index 69345b0458..8016b4edc2 100644 --- a/packages/desktop/src/main/store.ts +++ b/packages/desktop/src/main/store.ts @@ -1,35 +1,59 @@ -import Store from "electron-store" import electron from "electron" import { rmSync } from "node:fs" import { join } from "node:path" import { SETTINGS_STORE } from "./store-keys" import { deleteStoreFileIfEmpty } from "./store-cleanup" +import { createFileStore, type FileStore } from "./store-file" -const cache = new Map() +const cache = new Map() -// We cannot instantiate the electron-store at module load time because -// module import hoisting causes this to run before app.setPath("userData", ...) -// in index.ts has executed, which would result in files being written to the default directory -// (e.g. bad: %APPDATA%\@opencode-ai\desktop\opencode.settings vs good: %APPDATA%\ai.codev.desktop.dev\opencode.settings). +// We cannot instantiate a store at module load time because module import +// hoisting causes this to run before app.setPath("userData", ...) in index.ts +// has executed, which would result in files being written to the default +// directory (e.g. bad: %APPDATA%\@opencode-ai\desktop\opencode.settings vs +// good: %APPDATA%\ai.codev.desktop.dev\opencode.settings). +// +// Stores write asynchronously (see store-file.ts): a synchronous fsync on the +// main thread froze the whole app when a kernel journal stall never woke it, +// and renderers persist through store-set at exactly the busy moments (e.g. +// a sandbox restart). export function getStore(name = SETTINGS_STORE) { const cached = cache.get(name) if (cached) return cached - const next = new Store({ - name, - cwd: electron.app.getPath("userData"), - fileExtension: "", - accessPropertiesByDotNotation: false, + const next = createFileStore(join(electron.app.getPath("userData"), name), { + // Lazy: logging drags in electron named exports, which this module's + // test-importers cannot load outside the electron runtime. + onError: async (error) => { + const { write } = await import("./logging") + write("main", "store write failed", { name, error }, "error") + }, }) cache.set(name, next) return next } +// Awaited from stopSidecars so relaunch/exit paths keep pending settings; +// bounded so a wedged disk delays shutdown by at most the timeout instead of +// hanging it. Quit via window close stays fire-and-forget, matching the +// sidecar teardown it rides with. +export async function flushStoreWrites(timeoutMs: number) { + await Promise.all([...cache.values()].map((store) => store.writer.flush(timeoutMs))) +} + export async function removeStoreFileIfEmpty(name: string) { - if (await deleteStoreFileIfEmpty(electron.app.getPath("userData"), name)) cache.delete(name) + // The on-disk file may trail the in-memory state; settle it first so the + // emptiness check reads the truth. + await cache.get(name)?.writer.flush() + if (!(await deleteStoreFileIfEmpty(electron.app.getPath("userData"), name))) return + cache.get(name)?.writer.discard() + cache.delete(name) } export function removeStoreFile(name: string) { - rmSync(join(electron.app.getPath("userData"), name), { force: true }) + // Discard first: a pending or in-flight flush finishing after the rm would + // resurrect the file. + cache.get(name)?.writer.discard() cache.delete(name) + rmSync(join(electron.app.getPath("userData"), name), { force: true }) }