Repository navigation
fix(desktop): throttle settings writes and retry the temp-file rename - #556
Conversation
… temp files The renderer rewrote the whole thread list (350 KB of JSON, in client-settings.json) on every change, ~2 times a second at idle. On Windows ~30% of those replaces failed (the file was held for a moment) and each failure left its temp file behind: 19,102 files, 6.45 GB on one machine. - Renderer: persist the thread list at once on the first change, then at most every 2 s, and never the same content twice (flushed on pagehide) - Desktop: retry the replace while the settings file is held (EPERM/EBUSY/EACCES, not a directory), remove the temp file when it cannot be replaced, and clear stale temp files of earlier failed writes at start-up in the background Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
github-actions
Bot
added
vouch:trusted
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18f1d97cfb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Effect.flatMap((isDirectory) => | ||
| isDirectory | ||
| ? Effect.fail(error) | ||
| : Effect.sleep(delay).pipe(Effect.andThen(attempt(index + 1))), |
There was a problem hiding this comment.
When write A hits one of these transient errors, the sleep allows a later IPC write B to rename its temp file successfully because DesktopClientSettings.set calls are not serialized. A can then wake and rename its older snapshot over B, reverting every client-settings field changed by the newer request. Serialize the complete replace operation, or otherwise prevent an older retry from committing after a newer write.
Useful? React with 👍 / 👎.
| if (typeof window !== "undefined") { | ||
| window.addEventListener?.("pagehide", flushTrailingStoredThreads); |
There was a problem hiding this comment.
When the desktop window closes or reloads during the two-second throttle window, this handler only starts a fire-and-forget async flush. writeStoredThreadsJson must first await getClientSettings() before it can send setClientSettings(), but pagehide does not keep the renderer alive for that promise, so the latest thread snapshot can be lost and revert on the next launch. Flush through a renderer-close handshake or a main-process queue that survives renderer teardown.
Useful? React with 👍 / 👎.
Thread transfer impact
This comment will update automatically after the next completed run. |
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Fixes a confirmed write storm and file leak in the desktop client settings.
settings.jsonon every change. It is now throttled (2 s leading write, trailing coalesce, skip identical, flush onpagehide).DesktopClientSettingsrenamed its temp file with a single attempt, which fails on Windows with EPERM/EBUSY. It now retries, and stale temp files are removed at startup (about 19k had leaked).This does not prove the cause of the server freeze; that is covered by the stall profiler in #554.
Tested: new "write throttling" tests (fake timers) and
t3team-settingsFileReplace.test.ts.🤖 Generated with Claude Code